Skip to content

Commit f381640

Browse files
mjguzikbrauner
authored andcommitted
fs: consistently deref the files table with rcu_dereference_raw()
... except when the table is known to be only used by one thread. A file pointer can get installed at any moment despite the ->file_lock being held since the following: 8a81252 ("fs/file.c: don't acquire files->file_lock in fd_install()") Accesses subject to such a race can in principle suffer load tearing. While here redo the comment in dup_fd -- it only covered a race against files showing up, still assuming fd_install() takes the lock. Signed-off-by: Mateusz Guzik <mjguzik@gmail.com> Link: https://lore.kernel.org/r/20250313135725.1320914-1-mjguzik@gmail.com Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent 64a56f6 commit f381640

1 file changed

Lines changed: 17 additions & 9 deletions

File tree

fs/file.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -418,17 +418,25 @@ struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_ho
418418
old_fds = old_fdt->fd;
419419
new_fds = new_fdt->fd;
420420

421+
/*
422+
* We may be racing against fd allocation from other threads using this
423+
* files_struct, despite holding ->file_lock.
424+
*
425+
* alloc_fd() might have already claimed a slot, while fd_install()
426+
* did not populate it yet. Note the latter operates locklessly, so
427+
* the file can show up as we are walking the array below.
428+
*
429+
* At the same time we know no files will disappear as all other
430+
* operations take the lock.
431+
*
432+
* Instead of trying to placate userspace racing with itself, we
433+
* ref the file if we see it and mark the fd slot as unused otherwise.
434+
*/
421435
for (i = open_files; i != 0; i--) {
422-
struct file *f = *old_fds++;
436+
struct file *f = rcu_dereference_raw(*old_fds++);
423437
if (f) {
424438
get_file(f);
425439
} else {
426-
/*
427-
* The fd may be claimed in the fd bitmap but not yet
428-
* instantiated in the files array if a sibling thread
429-
* is partway through open(). So make sure that this
430-
* fd is available to the new process.
431-
*/
432440
__clear_open_fd(open_files - i, new_fdt);
433441
}
434442
rcu_assign_pointer(*new_fds++, f);
@@ -680,7 +688,7 @@ struct file *file_close_fd_locked(struct files_struct *files, unsigned fd)
680688
return NULL;
681689

682690
fd = array_index_nospec(fd, fdt->max_fds);
683-
file = fdt->fd[fd];
691+
file = rcu_dereference_raw(fdt->fd[fd]);
684692
if (file) {
685693
rcu_assign_pointer(fdt->fd[fd], NULL);
686694
__put_unused_fd(files, fd);
@@ -1248,7 +1256,7 @@ __releases(&files->file_lock)
12481256
*/
12491257
fdt = files_fdtable(files);
12501258
fd = array_index_nospec(fd, fdt->max_fds);
1251-
tofree = fdt->fd[fd];
1259+
tofree = rcu_dereference_raw(fdt->fd[fd]);
12521260
if (!tofree && fd_is_open(fd, fdt))
12531261
goto Ebusy;
12541262
get_file(file);

0 commit comments

Comments
 (0)