[PATCH V2] smb: client: fix use-after-free in cifs_oplock_break

[PATCH V2] smb: client: fix use-after-free in cifs_oplock_break

[Wang Zhaolong]

A race condition can occur in cifs_oplock_break() leading to a
use-after-free of the cinode structure when unmounting:

  cifs_oplock_break()
    _cifsFileInfo_put(cfile)
      cifsFileInfo_put_final()
        cifs_sb_deactive()
          [last ref, start releasing sb]
            kill_sb()
              kill_anon_super()
                generic_shutdown_super()
                  evict_inodes()
                    dispose_list()
                      evict()
                        destroy_inode()
                          call_rcu(&inode->i_rcu, i_callback)
    spin_lock(&cinode->open_file_lock)  <- OK
                            [later] i_callback()
                              cifs_free_inode()
                                kmem_cache_free(cinode)
    spin_unlock(&cinode->open_file_lock)  <- UAF
    cifs_done_oplock_break(cinode)       <- UAF

The issue occurs when umount has already released its reference to the
superblock. When _cifsFileInfo_put() calls cifs_sb_deactive(), this
releases the last reference, triggering the immediate cleanup of all
inodes under RCU. However, cifs_oplock_break() continues to access the
cinode after this point, resulting in use-after-free.

Fix this by holding an extra reference to the superblock during the
entire oplock break operation. This ensures that the superblock and
its inodes remain valid until the oplock break completes.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220309
Fixes: b98749cac4a6 ("CIFS: keep FileInfo handle live during oplock break")
Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
---
 fs/smb/client/file.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

V1 -> V2:
Correct the commit message call stack

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index e9212da32f01..1421bde045c2 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -3086,20 +3086,27 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
 void cifs_oplock_break(struct work_struct *work)
 {
 	struct cifsFileInfo *cfile = container_of(work, struct cifsFileInfo,
 						  oplock_break);
 	struct inode *inode = d_inode(cfile->dentry);
-	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+	struct super_block *sb = inode->i_sb;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifsInodeInfo *cinode = CIFS_I(inode);
 	struct cifs_tcon *tcon;
 	struct TCP_Server_Info *server;
 	struct tcon_link *tlink;
 	int rc = 0;
 	bool purge_cache = false, oplock_break_cancelled;
 	__u64 persistent_fid, volatile_fid;
 	__u16 net_fid;
 
+	/*
+	 * Hold a reference to the superblock to prevent it and its inodes from
+	 * being freed while we are accessing cinode. Otherwise, _cifsFileInfo_put()
+	 * may release the last reference to the sb and trigger inode eviction.
+	 */
+	cifs_sb_active(sb);
 	wait_on_bit(&cinode->flags, CIFS_INODE_PENDING_WRITERS,
 			TASK_UNINTERRUPTIBLE);
 
 	tlink = cifs_sb_tlink(cifs_sb);
 	if (IS_ERR(tlink))
@@ -3168,10 +3175,11 @@ void cifs_oplock_break(struct work_struct *work)
 		spin_unlock(&cinode->open_file_lock);
 
 	cifs_put_tlink(tlink);
 out:
 	cifs_done_oplock_break(cinode);
+	cifs_sb_deactive(sb);
 }
 
 static int cifs_swap_activate(struct swap_info_struct *sis,
 			      struct file *swap_file, sector_t *span)
 {
-- 
2.34.3

Re: [PATCH V2] smb: client: fix use-after-free in cifs_oplock_break

[Paulo Alcantara]

Wang Zhaolong <wangzhaolong@huaweicloud.com> writes:

> A race condition can occur in cifs_oplock_break() leading to a
> use-after-free of the cinode structure when unmounting:
>
>   cifs_oplock_break()
>     _cifsFileInfo_put(cfile)
>       cifsFileInfo_put_final()
>         cifs_sb_deactive()
>           [last ref, start releasing sb]
>             kill_sb()
>               kill_anon_super()
>                 generic_shutdown_super()
>                   evict_inodes()
>                     dispose_list()
>                       evict()
>                         destroy_inode()
>                           call_rcu(&inode->i_rcu, i_callback)
>     spin_lock(&cinode->open_file_lock)  <- OK
>                             [later] i_callback()
>                               cifs_free_inode()
>                                 kmem_cache_free(cinode)
>     spin_unlock(&cinode->open_file_lock)  <- UAF
>     cifs_done_oplock_break(cinode)       <- UAF
>
> The issue occurs when umount has already released its reference to the
> superblock. When _cifsFileInfo_put() calls cifs_sb_deactive(), this
> releases the last reference, triggering the immediate cleanup of all
> inodes under RCU. However, cifs_oplock_break() continues to access the
> cinode after this point, resulting in use-after-free.
>
> Fix this by holding an extra reference to the superblock during the
> entire oplock break operation. This ensures that the superblock and
> its inodes remain valid until the oplock break completes.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220309
> Fixes: b98749cac4a6 ("CIFS: keep FileInfo handle live during oplock break")
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>  fs/smb/client/file.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH V2] smb: client: fix use-after-free in cifs_oplock_break

[Steve French]

Good catch.  I had missed the patch because it was tagged by gmail as 'spam'

Merged into cifs-2.6.git for-next


On Fri, Jul 11, 2025 at 9:50 AM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Wang Zhaolong <wangzhaolong@huaweicloud.com> writes:
>
> > A race condition can occur in cifs_oplock_break() leading to a
> > use-after-free of the cinode structure when unmounting:
> >
> >   cifs_oplock_break()
> >     _cifsFileInfo_put(cfile)
> >       cifsFileInfo_put_final()
> >         cifs_sb_deactive()
> >           [last ref, start releasing sb]
> >             kill_sb()
> >               kill_anon_super()
> >                 generic_shutdown_super()
> >                   evict_inodes()
> >                     dispose_list()
> >                       evict()
> >                         destroy_inode()
> >                           call_rcu(&inode->i_rcu, i_callback)
> >     spin_lock(&cinode->open_file_lock)  <- OK
> >                             [later] i_callback()
> >                               cifs_free_inode()
> >                                 kmem_cache_free(cinode)
> >     spin_unlock(&cinode->open_file_lock)  <- UAF
> >     cifs_done_oplock_break(cinode)       <- UAF
> >
> > The issue occurs when umount has already released its reference to the
> > superblock. When _cifsFileInfo_put() calls cifs_sb_deactive(), this
> > releases the last reference, triggering the immediate cleanup of all
> > inodes under RCU. However, cifs_oplock_break() continues to access the
> > cinode after this point, resulting in use-after-free.
> >
> > Fix this by holding an extra reference to the superblock during the
> > entire oplock break operation. This ensures that the superblock and
> > its inodes remain valid until the oplock break completes.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=220309
> > Fixes: b98749cac4a6 ("CIFS: keep FileInfo handle live during oplock break")
> > Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> > ---
> >  fs/smb/client/file.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
>
> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
>


-- 
Thanks,

Steve