mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
smb: client: don't resurrect an exiting tcon from cached dir workers
cfids_laundromat_worker() bumps tcon->tc_count for every expired open
cfid and queues cached_dir_offload_close() without checking whether the
tcon is already being torn down. Racing with the final cifs_put_tcon(),
the tcon comes back from zero and gets torn down twice:
T1 T2
cifs_put_tcon()
--tc_count == 0, TID_EXITING
tconInfoFree()
free_cached_dirs()
cancel_delayed_work_sync() cfids_laundromat_worker()
++tc_count == 1
queue_work(serverclose_wq)
kfree(tcon->origin_fullpath)
kfree(tcon)
cached_dir_offload_close()
close_cached_dir() [UAF]
cifs_put_tcon()
tconInfoFree() [double free]
cached_dir_lease_break() also bumps tc_count without tc_lock, racing with
the laundromat's locked update.
Take the tcon reference through a helper that refuses once the tcon is
TID_EXITING or its refcount has dropped to zero. Queued put_work and
close_work then always hold a tcon reference, so tconInfoFree() can't
run before they finish and free_cached_dirs() doesn't need to flush any
workqueue. If the laundromat can't get a reference, put the cfid back on
the dying list for free_cached_dirs() to free; its handle goes away with
the tree connection.
The lease break path finds the tcon on ses->tcon_list under
cifs_tcp_ses_lock, so it can't see TID_EXITING, but it now takes tc_lock.
Fixes: 3fa640d035 ("smb: During unmount, ensure all cached dir instances drop their dentry")
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
This commit is contained in:
parent
7b1afb6313
commit
d58e7b4090
|
|
@ -649,6 +649,17 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon, bool sync)
|
|||
flush_delayed_work(&cfids->laundromat_work);
|
||||
}
|
||||
|
||||
static bool cached_dir_get_tcon(struct cifs_tcon *tcon,
|
||||
enum smb3_tcon_ref_trace trace)
|
||||
{
|
||||
guard(spinlock)(&tcon->tc_lock);
|
||||
if (tcon->status == TID_EXITING || tcon->tc_count <= 0)
|
||||
return false;
|
||||
++tcon->tc_count;
|
||||
trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count, trace);
|
||||
return true;
|
||||
}
|
||||
|
||||
static void
|
||||
cached_dir_offload_close(struct work_struct *work)
|
||||
{
|
||||
|
|
@ -693,6 +704,9 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
|
|||
!memcmp(lease_key,
|
||||
cfid->fid.lease_key,
|
||||
SMB2_LEASE_KEY_SIZE)) {
|
||||
if (!cached_dir_get_tcon(tcon,
|
||||
netfs_trace_tcon_ref_get_cached_lease_break))
|
||||
break;
|
||||
cfid->has_lease = false;
|
||||
cfid->time = 0;
|
||||
/*
|
||||
|
|
@ -703,9 +717,6 @@ bool cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16])
|
|||
cfid->on_list = false;
|
||||
cfids->num_entries--;
|
||||
|
||||
++tcon->tc_count;
|
||||
trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count,
|
||||
netfs_trace_tcon_ref_get_cached_lease_break);
|
||||
queue_work(cfid_put_wq, &cfid->put_work);
|
||||
spin_unlock(&cfids->cfid_list_lock);
|
||||
return true;
|
||||
|
|
@ -813,12 +824,13 @@ static void cfids_laundromat_worker(struct work_struct *work)
|
|||
cfid->dentry = NULL;
|
||||
|
||||
if (cfid->is_open) {
|
||||
spin_lock(&cfid->tcon->tc_lock);
|
||||
++cfid->tcon->tc_count;
|
||||
trace_smb3_tcon_ref(cfid->tcon->debug_id, cfid->tcon->tc_count,
|
||||
netfs_trace_tcon_ref_get_cached_laundromat);
|
||||
spin_unlock(&cfid->tcon->tc_lock);
|
||||
queue_work(serverclose_wq, &cfid->close_work);
|
||||
if (cached_dir_get_tcon(cfid->tcon,
|
||||
netfs_trace_tcon_ref_get_cached_laundromat)) {
|
||||
queue_work(serverclose_wq, &cfid->close_work);
|
||||
continue;
|
||||
}
|
||||
scoped_guard(spinlock, &cfids->cfid_list_lock)
|
||||
list_add_tail(&cfid->entry, &cfids->dying);
|
||||
} else
|
||||
/*
|
||||
* Drop the ref-count from above, either the lease-ref (if there
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user