mirrors
/
linux-stable
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
1
0
Fork 0
Code Issues Releases Wiki Activity

debugfs: fix wait/cancellation handling during remove 

Ben Greear further reports deadlocks during concurrent debugfs
remove while files are being accessed, even though the code in
question now uses debugfs cancellations. Turns out that despite
all the review on the locking, we missed completely that the
logic is wrong: if the refcount hits zero we can finish (and
need not wait for the completion), but if it doesn't we have
to trigger all the cancellations. As written, we can _never_
get into the loop triggering the cancellations. Fix this, and
explain it better while at it.

Cc: stable@vger.kernel.org
Fixes: 8c88a47435 ("debugfs: add API to allow debugfs operations cancellation")
Reported-by: Ben Greear <greearb@candelatech.com>
Closes: https://lore.kernel.org/r/1c9fa9e5-09f1-0522-fdbc-dbcef4d255ca@candelatech.com
Tested-by: Madhan Sai <madhan.singaraju@candelatech.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Link: https://lore.kernel.org/r/20240229153635.6bfab7eb34d3.I6c7aeff8c9d6628a8bc1ddcf332205a49d801f17@changeid
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Johannes Berg 2024-02-29 15:36:20 +01:00 committed by Greg Kroah-Hartman
parent 4dc3d612ee
commit 952c3fce29
1 changed files with 20 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
fs/debugfs/inode.c
View File

@ -751,13 +751,28 @@ static void __debugfs_file_removed(struct dentry *dentry)
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;
/* if we hit zero, just wait for all to finish */
if (!refcount_dec_and_test(&fsd->active_users)) {
wait_for_completion(&fsd->active_users_drained);
/* if this was the last reference, we're done */
if (refcount_dec_and_test(&fsd->active_users))
return;
}
/* if we didn't hit zero, try to cancel any we can */
/*
* If there's still a reference, the code that obtained it can
* be in different states:
* - The common case of not using cancellations, or already
* after debugfs_leave_cancellation(), where we just need
* to wait for debugfs_file_put() which signals the completion;
* - inside a cancellation section, i.e. between
* debugfs_enter_cancellation() and debugfs_leave_cancellation(),
* in which case we need to trigger the ->cancel() function,
* and then wait for debugfs_file_put() just like in the
* previous case;
* - before debugfs_enter_cancellation() (but obviously after
* debugfs_file_get()), in which case we may not see the
* cancellation in the list on the first round of the loop,
* but debugfs_enter_cancellation() signals the completion
* after adding it, so this code gets woken up to call the
* ->cancel() function.
*/
while (refcount_read(&fsd->active_users)) {
struct debugfs_cancellation *c;