[syzbot] [fs?] [wireless?] general protection fault in simple_recursive_removal (5)

Moon Hee Lee posted 1 patch 2 months ago
net/mac80211/debugfs_netdev.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
[syzbot] [fs?] [wireless?] general protection fault in simple_recursive_removal (5)
Posted by Moon Hee Lee 2 months ago
#syz test git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main


Thanks for the review and valuable feedback.

Upon investigation, I found the crash occurs when the netdev's debugfs
directory is removed while a station still holds a pointer
(sta->debugfs_dir) to a dentry within it. A subsequent call to
ieee80211_sta_debugfs_remove() may then dereference a freed dentry,
triggering a use-after-free.

To address this, I’m preparing a patch that clears sta->debugfs_dir for
all stations associated with the interface before calling
debugfs_remove_recursive(). This ensures any later station removal
becomes a no-op and avoids referencing a stale pointer.

This reply is intended for syz testing and to provide context for
review. A formal patch will follow.

Many thanks to Hillf Danton and Al Viro for their insights.

---
 net/mac80211/debugfs_netdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 1dac78271045..4d45bb4fe380 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -1015,9 +1015,24 @@ static void ieee80211_debugfs_add_netdev(struct ieee80211_sub_if_data *sdata,
 
 void ieee80211_debugfs_remove_netdev(struct ieee80211_sub_if_data *sdata)
 {
+	struct sta_info *sta;
+
 	if (!sdata->vif.debugfs_dir)
 		return;
 
+	/*
+	 * Before we delete the netdev’s debugfs tree, clear sta->debugfs_dir
+	 * for every station on this interface.  This ensures any later call to
+	 * ieee80211_sta_debugfs_remove() sees NULL and avoids touching a dentry
+	 * that we are about to free.
+	 */
+	rcu_read_lock();
+	list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
+		if (sta->sdata == sdata)
+			sta->debugfs_dir = NULL;
+	}
+	rcu_read_unlock();
+
 	debugfs_remove_recursive(sdata->vif.debugfs_dir);
 	sdata->vif.debugfs_dir = NULL;
 	sdata->debugfs.subdir_stations = NULL;
-- 
2.43.0

Re: [syzbot] [fs?] [wireless?] general protection fault in simple_recursive_removal (5)
Posted by syzbot 2 months ago
Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+d6ccd49ae046542a0641@syzkaller.appspotmail.com
Tested-by: syzbot+d6ccd49ae046542a0641@syzkaller.appspotmail.com

Tested on:

commit:         126d85fb Merge tag 'wireless-next-2025-07-24' of https..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
console output: https://syzkaller.appspot.com/x/log.txt?x=1104acf0580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=4e0aa2ee2717f461
dashboard link: https://syzkaller.appspot.com/bug?extid=d6ccd49ae046542a0641
compiler:       Debian clang version 20.1.7 (++20250616065708+6146a88f6049-1~exp1~20250616065826.132), Debian LLD 20.1.7
patch:          https://syzkaller.appspot.com/x/patch.diff?x=128ee9bc580000

Note: testing is done by a robot and is best-effort only.
Re: [syzbot] [fs?] [wireless?] general protection fault in simple_recursive_removal (5)
Posted by Al Viro 2 months ago
On Thu, Jul 31, 2025 at 10:17:29AM -0700, Moon Hee Lee wrote:
> #syz test git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
> 
> 
> Thanks for the review and valuable feedback.
> 
> Upon investigation, I found the crash occurs when the netdev's debugfs
> directory is removed while a station still holds a pointer
> (sta->debugfs_dir) to a dentry within it. A subsequent call to
> ieee80211_sta_debugfs_remove() may then dereference a freed dentry,
> triggering a use-after-free.
> 
> To address this, I’m preparing a patch that clears sta->debugfs_dir for
> all stations associated with the interface before calling
> debugfs_remove_recursive(). This ensures any later station removal
> becomes a no-op and avoids referencing a stale pointer.
> 
> This reply is intended for syz testing and to provide context for
> review. A formal patch will follow.

> +	/*
> +	 * Before we delete the netdev’s debugfs tree, clear sta->debugfs_dir
> +	 * for every station on this interface.  This ensures any later call to
> +	 * ieee80211_sta_debugfs_remove() sees NULL and avoids touching a dentry
> +	 * that we are about to free.
> +	 */
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
> +		if (sta->sdata == sdata)
> +			sta->debugfs_dir = NULL;
> +	}
> +	rcu_read_unlock();

Umm... Is there any exclusion between that an ieee80211_sta_debugfs_remove()?
This looks fishy...