Skip to content

Commit a9d1f38

Browse files
hac-vsmfrench
authored andcommitted
smb: client: introduce close_cached_dir_locked()
Replace close_cached_dir() calls under cfid_list_lock with a new close_cached_dir_locked() variant that uses kref_put() instead of kref_put_lock() to avoid recursive locking when dropping references. While the existing code works if the refcount >= 2 invariant holds, this area has proven error-prone. Make deadlocks impossible and WARN on invariant violations. Cc: [email protected] Reviewed-by: David Howells <[email protected]> Signed-off-by: Henrique Carvalho <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 6a23ae0 commit a9d1f38

File tree

1 file changed

+38
-3
lines changed

1 file changed

+38
-3
lines changed

fs/smb/client/cached_dir.c

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ static struct cached_fid *init_cached_dir(const char *path);
1616
static void free_cached_dir(struct cached_fid *cfid);
1717
static void smb2_close_cached_fid(struct kref *ref);
1818
static void cfids_laundromat_worker(struct work_struct *work);
19+
static void close_cached_dir_locked(struct cached_fid *cfid);
1920

2021
struct cached_dir_dentry {
2122
struct list_head entry;
@@ -388,7 +389,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
388389
* lease. Release one here, and the second below.
389390
*/
390391
cfid->has_lease = false;
391-
close_cached_dir(cfid);
392+
close_cached_dir_locked(cfid);
392393
}
393394
spin_unlock(&cfids->cfid_list_lock);
394395

@@ -480,18 +481,52 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
480481
spin_lock(&cfid->cfids->cfid_list_lock);
481482
if (cfid->has_lease) {
482483
cfid->has_lease = false;
483-
close_cached_dir(cfid);
484+
close_cached_dir_locked(cfid);
484485
}
485486
spin_unlock(&cfid->cfids->cfid_list_lock);
486487
close_cached_dir(cfid);
487488
}
488489

489-
490+
/**
491+
* close_cached_dir - drop a reference of a cached dir
492+
*
493+
* The release function will be called with cfid_list_lock held to remove the
494+
* cached dirs from the list before any other thread can take another @cfid
495+
* ref. Must not be called with cfid_list_lock held; use
496+
* close_cached_dir_locked() called instead.
497+
*
498+
* @cfid: cached dir
499+
*/
490500
void close_cached_dir(struct cached_fid *cfid)
491501
{
502+
lockdep_assert_not_held(&cfid->cfids->cfid_list_lock);
492503
kref_put_lock(&cfid->refcount, smb2_close_cached_fid, &cfid->cfids->cfid_list_lock);
493504
}
494505

506+
/**
507+
* close_cached_dir_locked - put a reference of a cached dir with
508+
* cfid_list_lock held
509+
*
510+
* Calling close_cached_dir() with cfid_list_lock held has the potential effect
511+
* of causing a deadlock if the invariant of refcount >= 2 is false.
512+
*
513+
* This function is used in paths that hold cfid_list_lock and expect at least
514+
* two references. If that invariant is violated, WARNs and returns without
515+
* dropping a reference; the final put must still go through
516+
* close_cached_dir().
517+
*
518+
* @cfid: cached dir
519+
*/
520+
static void close_cached_dir_locked(struct cached_fid *cfid)
521+
{
522+
lockdep_assert_held(&cfid->cfids->cfid_list_lock);
523+
524+
if (WARN_ON(kref_read(&cfid->refcount) < 2))
525+
return;
526+
527+
kref_put(&cfid->refcount, smb2_close_cached_fid);
528+
}
529+
495530
/*
496531
* Called from cifs_kill_sb when we unmount a share
497532
*/

0 commit comments

Comments
 (0)