linux-kernel - Re: [PATCH AUTOSEL 6.6 15/32] debugfs: annotate debugfs handlers vs. removal with lockdep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a7a8caa3fe9b4e3271239b86ebd24a41464b79f.camel@sipsolutions.net>
Date:   Mon, 04 Dec 2023 21:45:55 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Sasha Levin <sashal@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH AUTOSEL 6.6 15/32] debugfs: annotate debugfs handlers
 vs. removal with lockdep

On Mon, 2023-12-04 at 20:32 +0000, Sasha Levin wrote:
> From: Johannes Berg <johannes.berg@...el.com>
> 
> [ Upstream commit f4acfcd4deb158b96595250cc332901b282d15b0 ]
> 
> When you take a lock in a debugfs handler but also try
> to remove the debugfs file under that lock, things can
> deadlock since the removal has to wait for all users
> to finish.
> 
> Add lockdep annotations in debugfs_file_get()/_put()
> to catch such issues.
> 

This (and the previous patch) probably got picked up as dependencies for
the locking things, but ... we reverted this.

For 6.6, _maybe_ it's worth backporting this including the revert, but
then I'd do that only when the revert landed to have them together. But
then you should apply all the six patches listed below _and_ the revert,
the set as here doesn't do anything useful.

However ... given that debugfs is root-only, and you have to be
reading/writing a file _while_ disconnecting and the file is removed,
perhaps the whole thing isn't worth backporting at all.

For 6.1 and earlier, I believe it's not needed at all, so please drop
from there all of these:

 - debugfs: fix automount d_fsdata usage
 - debugfs: annotate debugfs handlers vs. removal with lockdep
 - debugfs: add API to allow debugfs operations cancellation
 - wifi: cfg80211: add locked debugfs wrappers
 - wifi: mac80211: use wiphy locked debugfs helpers for agg_status
 - wifi: mac80211: use wiphy locked debugfs for sdata/link

I'd kind of think just dropping all of these completely makes more
sense.

johannes