Skip to content

Commit bdb596c

Browse files
hac-vgregkh
authored andcommitted
smb: client: fix potential UAF in smb2_close_cached_fid()
commit 734e99623c5b65bf2c03e35978a0b980ebc3c2f8 upstream. find_or_create_cached_dir() could grab a new reference after kref_put() had seen the refcount drop to zero but before cfid_list_lock is acquired in smb2_close_cached_fid(), leading to use-after-free. Switch to kref_put_lock() so cfid_release() is called with cfid_list_lock held, closing that gap. Fixes: ebe98f1 ("cifs: enable caching of directories for which a lease is held") Cc: [email protected] Reported-by: Jay Shin <[email protected]> Reviewed-by: Paulo Alcantara (Red Hat) <[email protected]> Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 826ce37 commit bdb596c

1 file changed

Lines changed: 9 additions & 7 deletions

File tree

fs/smb/client/cached_dir.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -389,11 +389,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
389389
* lease. Release one here, and the second below.
390390
*/
391391
cfid->has_lease = false;
392-
kref_put(&cfid->refcount, smb2_close_cached_fid);
392+
close_cached_dir(cfid);
393393
}
394394
spin_unlock(&cfids->cfid_list_lock);
395395

396-
kref_put(&cfid->refcount, smb2_close_cached_fid);
396+
close_cached_dir(cfid);
397397
} else {
398398
*ret_cfid = cfid;
399399
atomic_inc(&tcon->num_remote_opens);
@@ -434,12 +434,14 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
434434

435435
static void
436436
smb2_close_cached_fid(struct kref *ref)
437+
__releases(&cfid->cfids->cfid_list_lock)
437438
{
438439
struct cached_fid *cfid = container_of(ref, struct cached_fid,
439440
refcount);
440441
int rc;
441442

442-
spin_lock(&cfid->cfids->cfid_list_lock);
443+
lockdep_assert_held(&cfid->cfids->cfid_list_lock);
444+
443445
if (cfid->on_list) {
444446
list_del(&cfid->entry);
445447
cfid->on_list = false;
@@ -474,7 +476,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
474476
spin_lock(&cfid->cfids->cfid_list_lock);
475477
if (cfid->has_lease) {
476478
cfid->has_lease = false;
477-
kref_put(&cfid->refcount, smb2_close_cached_fid);
479+
close_cached_dir(cfid);
478480
}
479481
spin_unlock(&cfid->cfids->cfid_list_lock);
480482
close_cached_dir(cfid);
@@ -483,7 +485,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
483485

484486
void close_cached_dir(struct cached_fid *cfid)
485487
{
486-
kref_put(&cfid->refcount, smb2_close_cached_fid);
488+
kref_put_lock(&cfid->refcount, smb2_close_cached_fid, &cfid->cfids->cfid_list_lock);
487489
}
488490

489491
/*
@@ -594,7 +596,7 @@ cached_dir_offload_close(struct work_struct *work)
594596

595597
WARN_ON(cfid->on_list);
596598

597-
kref_put(&cfid->refcount, smb2_close_cached_fid);
599+
close_cached_dir(cfid);
598600
cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close);
599601
}
600602

@@ -771,7 +773,7 @@ static void cfids_laundromat_worker(struct work_struct *work)
771773
* Drop the ref-count from above, either the lease-ref (if there
772774
* was one) or the extra one acquired.
773775
*/
774-
kref_put(&cfid->refcount, smb2_close_cached_fid);
776+
close_cached_dir(cfid);
775777
}
776778
queue_delayed_work(cfid_put_wq, &cfids->laundromat_work,
777779
dir_cache_timeout * HZ);

0 commit comments

Comments
 (0)