Skip to content

Commit 81ba109

Browse files
Paulo AlcantaraSteve French
authored andcommitted
smb: client: prevent new fids from being removed by laundromat
Check if @cfid->time is set in laundromat so we guarantee that only fully cached fids will be selected for removal. While we're at it, add missing locks to protect access of @cfid fields in order to avoid races with open_cached_dir() and cfids_laundromat_worker(), respectively. Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent e95f3f7 commit 81ba109

File tree

1 file changed

+35
-21
lines changed

1 file changed

+35
-21
lines changed

fs/smb/client/cached_dir.c

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -170,15 +170,18 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
170170
return -ENOENT;
171171
}
172172
/*
173-
* At this point we either have a lease already and we can just
174-
* return it. If not we are guaranteed to be the only thread accessing
175-
* this cfid.
173+
* Return cached fid if it has a lease. Otherwise, it is either a new
174+
* entry or laundromat worker removed it from @cfids->entries. Caller
175+
* will put last reference if the latter.
176176
*/
177+
spin_lock(&cfids->cfid_list_lock);
177178
if (cfid->has_lease) {
179+
spin_unlock(&cfids->cfid_list_lock);
178180
*ret_cfid = cfid;
179181
kfree(utf16_path);
180182
return 0;
181183
}
184+
spin_unlock(&cfids->cfid_list_lock);
182185

183186
/*
184187
* Skip any prefix paths in @path as lookup_positive_unlocked() ends up
@@ -295,9 +298,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
295298
goto oshr_free;
296299
}
297300
}
301+
spin_lock(&cfids->cfid_list_lock);
298302
cfid->dentry = dentry;
299303
cfid->time = jiffies;
300304
cfid->has_lease = true;
305+
spin_unlock(&cfids->cfid_list_lock);
301306

302307
oshr_free:
303308
kfree(utf16_path);
@@ -306,32 +311,36 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
306311
free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
307312
free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
308313
spin_lock(&cfids->cfid_list_lock);
309-
if (rc && !cfid->has_lease) {
310-
if (cfid->on_list) {
311-
list_del(&cfid->entry);
312-
cfid->on_list = false;
313-
cfids->num_entries--;
314+
if (!cfid->has_lease) {
315+
if (rc) {
316+
if (cfid->on_list) {
317+
list_del(&cfid->entry);
318+
cfid->on_list = false;
319+
cfids->num_entries--;
320+
}
321+
rc = -ENOENT;
322+
} else {
323+
/*
324+
* We are guaranteed to have two references at this
325+
* point. One for the caller and one for a potential
326+
* lease. Release the Lease-ref so that the directory
327+
* will be closed when the caller closes the cached
328+
* handle.
329+
*/
330+
spin_unlock(&cfids->cfid_list_lock);
331+
kref_put(&cfid->refcount, smb2_close_cached_fid);
332+
goto out;
314333
}
315-
rc = -ENOENT;
316334
}
317335
spin_unlock(&cfids->cfid_list_lock);
318-
if (!rc && !cfid->has_lease) {
319-
/*
320-
* We are guaranteed to have two references at this point.
321-
* One for the caller and one for a potential lease.
322-
* Release the Lease-ref so that the directory will be closed
323-
* when the caller closes the cached handle.
324-
*/
325-
kref_put(&cfid->refcount, smb2_close_cached_fid);
326-
}
327336
if (rc) {
328337
if (cfid->is_open)
329338
SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
330339
cfid->fid.volatile_fid);
331340
free_cached_dir(cfid);
332341
cfid = NULL;
333342
}
334-
343+
out:
335344
if (rc == 0) {
336345
*ret_cfid = cfid;
337346
atomic_inc(&tcon->num_remote_opens);
@@ -583,15 +592,18 @@ static void cfids_laundromat_worker(struct work_struct *work)
583592

584593
spin_lock(&cfids->cfid_list_lock);
585594
list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
586-
if (time_after(jiffies, cfid->time + HZ * dir_cache_timeout)) {
595+
if (cfid->time &&
596+
time_after(jiffies, cfid->time + HZ * dir_cache_timeout)) {
597+
cfid->on_list = false;
587598
list_move(&cfid->entry, &entry);
588599
cfids->num_entries--;
600+
/* To prevent race with smb2_cached_lease_break() */
601+
kref_get(&cfid->refcount);
589602
}
590603
}
591604
spin_unlock(&cfids->cfid_list_lock);
592605

593606
list_for_each_entry_safe(cfid, q, &entry, entry) {
594-
cfid->on_list = false;
595607
list_del(&cfid->entry);
596608
/*
597609
* Cancel and wait for the work to finish in case we are racing
@@ -608,6 +620,8 @@ static void cfids_laundromat_worker(struct work_struct *work)
608620
spin_unlock(&cfids->cfid_list_lock);
609621
kref_put(&cfid->refcount, smb2_close_cached_fid);
610622
}
623+
/* Drop the extra reference opened above */
624+
kref_put(&cfid->refcount, smb2_close_cached_fid);
611625
}
612626
queue_delayed_work(cifsiod_wq, &cfids->laundromat_work,
613627
dir_cache_timeout * HZ);

0 commit comments

Comments
 (0)