Add error checking to readdir, telldir, and closedir calls in dir.c

Raise SystemCallError exception when these functions return an error.

This changes behavior for the following case (found by the tests):

```ruby
dir1 = Dir.new('..')
dir2 = Dir.for_fd(dir1.fileno)
dir1.close
dir2.close
```

The above code is basically broken, as `dir1.close` closed the file
descriptor.  The subsequent `dir2.close` call is undefined behavior.
When run in isolation, it raises Errno::EBADF after the change, but
if another thread opens a file descriptor between the `dir1.close`
and `dir2.close` calls, the `dir2.close` call could close the file
descriptor opened by the other thread.  Raising an exception is much
better in this case as it makes it obvious there is a bug in the code.

For the readdir check, since the GVL has already been released,
reacquire it rb_thread_call_with_gvl if an exception needs to be
raised.

Due to the number of closedir calls, this adds static close_dir_data
and check_closedir functions.  The close_dir_data takes a
struct dir_data * and handles setting the dir entry to NULL regardless
of failure.

Fixes [Bug #20586]

Co-authored-by: Nobuyoshi Nakada <nobu.nakada@gmail.com>
This commit is contained in:
Jeremy Evans 2024-09-12 10:04:10 -07:00 коммит произвёл GitHub
Родитель 15135030e5
Коммит f2919bd11c
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
4 изменённых файлов: 48 добавлений и 15 удалений

54
dir.c
Просмотреть файл

@ -571,6 +571,25 @@ opendir_without_gvl(const char *path)
return opendir(path);
}
static void
close_dir_data(struct dir_data *dp)
{
if (dp->dir) {
if (closedir(dp->dir) < 0) {
dp->dir = NULL;
rb_sys_fail("closedir");
}
dp->dir = NULL;
}
}
static void
check_closedir(DIR *dirp)
{
if (closedir(dirp) < 0)
rb_sys_fail("closedir");
}
static VALUE
dir_initialize(rb_execution_context_t *ec, VALUE dir, VALUE dirname, VALUE enc)
{
@ -585,8 +604,7 @@ dir_initialize(rb_execution_context_t *ec, VALUE dir, VALUE dirname, VALUE enc)
dirname = rb_str_dup_frozen(dirname);
TypedData_Get_Struct(dir, struct dir_data, &dir_data_type, dp);
if (dp->dir) closedir(dp->dir);
dp->dir = NULL;
close_dir_data(dp);
RB_OBJ_WRITE(dir, &dp->path, Qnil);
dp->enc = fsenc;
path = RSTRING_PTR(dirname);
@ -810,10 +828,22 @@ fundamental_encoding_p(rb_encoding *enc)
# define READDIR(dir, enc) rb_w32_readdir((dir), (enc))
# define READDIR_NOGVL READDIR
#else
NORETURN(static void *sys_failure(void *function));
static void *
sys_failure(void *function)
{
rb_sys_fail(function);
}
static void *
nogvl_readdir(void *dir)
{
return readdir(dir);
rb_errno_set(0);
if ((dir = readdir(dir)) == NULL) {
if (rb_errno())
rb_thread_call_with_gvl(sys_failure, (void *)"readdir");
}
return dir;
}
# define READDIR(dir, enc) IO_WITHOUT_GVL(nogvl_readdir, (void *)(dir))
@ -962,7 +992,8 @@ dir_tell(VALUE dir)
long pos;
GetDIR(dir, dirp);
pos = telldir(dirp->dir);
if((pos = telldir(dirp->dir)) < 0)
rb_sys_fail("telldir");
return rb_int2inum(pos);
}
#else
@ -1081,8 +1112,7 @@ dir_close(VALUE dir)
dirp = dir_get(dir);
if (!dirp->dir) return Qnil;
closedir(dirp->dir);
dirp->dir = NULL;
close_dir_data(dirp);
return Qnil;
}
@ -2585,7 +2615,7 @@ static void
glob_dir_finish(ruby_glob_entries_t *ent, int flags)
{
if (flags & FNM_GLOB_NOSORT) {
closedir(ent->nosort.dirp);
check_closedir(ent->nosort.dirp);
ent->nosort.dirp = NULL;
}
else if (ent->sort.entries) {
@ -2616,7 +2646,7 @@ glob_opendir(ruby_glob_entries_t *ent, DIR *dirp, int flags, rb_encoding *enc)
#ifdef _WIN32
if ((capacity = dirp->nfiles) > 0) {
if (!(newp = GLOB_ALLOC_N(rb_dirent_t, capacity))) {
closedir(dirp);
check_closedir(dirp);
return NULL;
}
ent->sort.entries = newp;
@ -2636,7 +2666,7 @@ glob_opendir(ruby_glob_entries_t *ent, DIR *dirp, int flags, rb_encoding *enc)
ent->sort.entries[count++] = rdp;
ent->sort.count = count;
}
closedir(dirp);
check_closedir(dirp);
if (count < capacity) {
if (!(newp = GLOB_REALLOC_N(ent->sort.entries, count))) {
glob_dir_finish(ent, 0);
@ -2651,7 +2681,7 @@ glob_opendir(ruby_glob_entries_t *ent, DIR *dirp, int flags, rb_encoding *enc)
nomem:
glob_dir_finish(ent, 0);
closedir(dirp);
check_closedir(dirp);
return NULL;
}
@ -2814,7 +2844,7 @@ glob_helper(
# if NORMALIZE_UTF8PATH
if (!(norm_p || magical || recursive)) {
closedir(dirp);
check_closedir(dirp);
goto literally;
}
# endif
@ -3712,7 +3742,7 @@ nogvl_dir_empty_p(void *ptr)
break;
}
}
closedir(dir);
check_closedir(dir);
return (void *)result;
}

Просмотреть файл

@ -710,7 +710,9 @@ class TestDir < Test::Unit::TestCase
assert_equal(new_dir.chdir{Dir.pwd}, for_fd_dir.chdir{Dir.pwd})
ensure
new_dir&.close
for_fd_dir&.close
if for_fd_dir
assert_raise(Errno::EBADF) { for_fd_dir.close }
end
end
else
assert_raise(NotImplementedError) { Dir.for_fd(0) }

Просмотреть файл

@ -36,7 +36,7 @@ struct direct* rb_w32_ureaddir(DIR *);
long rb_w32_telldir(DIR *);
void rb_w32_seekdir(DIR *, long);
void rb_w32_rewinddir(DIR *);
void rb_w32_closedir(DIR *);
int rb_w32_closedir(DIR *);
char *rb_w32_ugetcwd(char *, int);
#define opendir(s) rb_w32_uopendir((s))

Просмотреть файл

@ -2461,7 +2461,7 @@ rb_w32_rewinddir(DIR *dirp)
//
/* License: Artistic or GPL */
void
int
rb_w32_closedir(DIR *dirp)
{
if (dirp) {
@ -2471,6 +2471,7 @@ rb_w32_closedir(DIR *dirp)
free(dirp->bits);
free(dirp);
}
return 0;
}
#if RUBY_MSVCRT_VERSION >= 140