Patch "debugfs: fix wait/cancellation handling during remove" has been added to the 6.7-stable tree

[Sasha Levin <sashal@xxxxxxxxxx>]

This is a note to let you know that I've just added the patch titled

    debugfs: fix wait/cancellation handling during remove

to the 6.7-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     debugfs-fix-wait-cancellation-handling-during-remove.patch
and it can be found in the queue-6.7 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 01fad6fb0eed9f533665185f4144a019b4a74038
Author: Johannes Berg <johannes.berg@xxxxxxxxx>
Date:   Thu Feb 29 15:36:20 2024 +0100

    debugfs: fix wait/cancellation handling during remove
    
    [ Upstream commit 952c3fce297f12c7ff59380adb66b564e2bc9b64 ]
    
    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@xxxxxxxxxxxxxxx
    Fixes: 8c88a474357e ("debugfs: add API to allow debugfs operations cancellation")
    Reported-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
    Closes: https://lore.kernel.org/r/1c9fa9e5-09f1-0522-fdbc-dbcef4d255ca@xxxxxxxxxxxxxxx
    Tested-by: Madhan Sai <madhan.singaraju@xxxxxxxxxxxxxxx>
    Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
    Link: https://lore.kernel.org/r/20240229153635.6bfab7eb34d3.I6c7aeff8c9d6628a8bc1ddcf332205a49d801f17@changeid
    Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 034a617cb1a5e..a40da00654336 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -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;