ksmbd: Fix UAF in __close_file_table_ids

A use-after-free is possible if one thread destroys the file
via __ksmbd_close_fd while another thread holds a reference to
it. The existing checks on fp->refcount are not sufficient to
prevent this.

The fix takes ft->lock around the section which removes the
file from the file table. This prevents two threads acquiring the
same file pointer via __close_file_table_ids, as well as the other
functions which retrieve a file from the IDR and which already use
this same lock.

Cc: stable@vger.kernel.org
Signed-off-by: Sean Heelan <seanheelan@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 1f8fa3468173ab06de64c3f2e33ab2824449c32c..dfed6fce890498995554cbc307e1db1189bb5c37 100644 (file)
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -661,21 +661,40 @@ __close_file_table_ids(struct ksmbd_file_table *ft,
                       bool (*skip)(struct ksmbd_tree_connect *tcon,
                                    struct ksmbd_file *fp))
 {
-       unsigned int                    id;
-       struct ksmbd_file               *fp;
-       int                             num = 0;
+       struct ksmbd_file *fp;
+       unsigned int id = 0;
+       int num = 0;
+
+       while (1) {
+               write_lock(&ft->lock);
+               fp = idr_get_next(ft->idr, &id);
+               if (!fp) {
+                       write_unlock(&ft->lock);
+                       break;
+               }
 
-       idr_for_each_entry(ft->idr, fp, id) {
-               if (skip(tcon, fp))
+               if (skip(tcon, fp) ||
+                   !atomic_dec_and_test(&fp->refcount)) {
+                       id++;
+                       write_unlock(&ft->lock);
                        continue;
+               }
 
                set_close_state_blocked_works(fp);
+               idr_remove(ft->idr, fp->volatile_id);
+               fp->volatile_id = KSMBD_NO_FID;
+               write_unlock(&ft->lock);
+
+               down_write(&fp->f_ci->m_lock);
+               list_del_init(&fp->node);
+               up_write(&fp->f_ci->m_lock);
 
-               if (!atomic_dec_and_test(&fp->refcount))
-                       continue;
                __ksmbd_close_fd(ft, fp);
+
                num++;
+               id++;
        }
+
        return num;
 }