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:
8a81252b774b53e6 ("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>

diff --git a/fs/file.c b/fs/file.c
index c8214975e24835c17fc7e7678387ec4c9124157e..2d66816af1058c8c4ab2dd3d87566307986aa2db 100644 (file)
--- a/fs/file.c
+++ b/fs/file.c
@@ -418,17 +418,25 @@ struct files_struct *dup_fd(struct files_struct *oldf, struct fd_range *punch_ho
        old_fds = old_fdt->fd;
        new_fds = new_fdt->fd;
 
+       /*
+        * We may be racing against fd allocation from other threads using this
+        * files_struct, despite holding ->file_lock.
+        *
+        * alloc_fd() might have already claimed a slot, while fd_install()
+        * did not populate it yet. Note the latter operates locklessly, so
+        * the file can show up as we are walking the array below.
+        *
+        * At the same time we know no files will disappear as all other
+        * operations take the lock.
+        *
+        * Instead of trying to placate userspace racing with itself, we
+        * ref the file if we see it and mark the fd slot as unused otherwise.
+        */
        for (i = open_files; i != 0; i--) {
-               struct file *f = *old_fds++;
+               struct file *f = rcu_dereference_raw(*old_fds++);
                if (f) {
                        get_file(f);
                } else {
-                       /*
-                        * The fd may be claimed in the fd bitmap but not yet
-                        * instantiated in the files array if a sibling thread
-                        * is partway through open().  So make sure that this
-                        * fd is available to the new process.
-                        */
                        __clear_open_fd(open_files - i, new_fdt);
                }
                rcu_assign_pointer(*new_fds++, f);
@@ -680,7 +688,7 @@ struct file *file_close_fd_locked(struct files_struct *files, unsigned fd)
                return NULL;
 
        fd = array_index_nospec(fd, fdt->max_fds);
-       file = fdt->fd[fd];
+       file = rcu_dereference_raw(fdt->fd[fd]);
        if (file) {
                rcu_assign_pointer(fdt->fd[fd], NULL);
                __put_unused_fd(files, fd);
@@ -1248,7 +1256,7 @@ __releases(&files->file_lock)
         */
        fdt = files_fdtable(files);
        fd = array_index_nospec(fd, fdt->max_fds);
-       tofree = fdt->fd[fd];
+       tofree = rcu_dereference_raw(fdt->fd[fd]);
        if (!tofree && fd_is_open(fd, fdt))
                goto Ebusy;
        get_file(file);