Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <c304ad9c-f404-d22e-de74-9398da3ebfc3@hauke-m.de>
In-Reply-To: <c304ad9c-f404-d22e-de74-9398da3ebfc3@hauke-m.de>
From:   Sumit Garg <sumit.garg@linaro.org>
Date:   Thu, 23 Apr 2020 13:04:26 +0530
Message-ID: <CAFA6WYN3FbqTivGJTfXtHsMjXNPXW+P4MZWiCL14utF2sHkeYg@mail.gmail.com>
Subject: Re: Commit "mac80211: fix race in ieee80211_register_hw()" breaks
 mac80211 debugfs
To:     Hauke Mehrtens <hauke@hauke-m.de>
Cc:     linux-wireless <linux-wireless@vger.kernel.org>,
        Felix Fietkau <nbd@nbd.name>, stable <stable@vger.kernel.org>,
        Johannes Berg <johannes.berg@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Thu, 23 Apr 2020 at 02:29, Hauke Mehrtens <hauke@hauke-m.de> wrote:
>
> Hi,
>
> Since commit 52e04b4ce5d0 ("mac80211: fix race in
> ieee80211_register_hw()") the debugfs entries for mac80211 drivers are
> broken.
> https://git.kernel.org/linus/52e04b4ce5d03775b6a78f3ed1097480faacc9fd
>
> Felix reported that the file /sys/kernel/debug/ieee80211/phy0/rc is now
> located at /sys/kernel/debug/rc.
>
> Before this commit we had the following flow:
> 1. wiphy_register()
>   -> creates /sys/kernel/debug/ieee80211/phy0/
>   -> fill rdev->wiphy.debugfsdir pointer
> 2. ieee80211_init_rate_ctrl_alg()
>   -> call rate_control_alloc()
>     -> use rdev->wiphy.debugfsdir pointer to
>        create /sys/kernel/debug/ieee80211/phy0/rc/
>
> This works like expected.
>
>
> With the commit the flow in ieee80211_register_hw() is the other way around:
> 2. ieee80211_init_rate_ctrl_alg()
>   -> call rate_control_alloc()
>     -> use rdev->wiphy.debugfsdir pointer (now NULL) to
>        create /sys/kernel/debug/rc/
> 2. wiphy_register()
>   -> creates /sys/kernel/debug/ieee80211/phy0/
>   -> fill rdev->wiphy.debugfsdir pointer
>

Thanks for the detailed report. I missed to notice this hidden debugfs
dependency on "wiphy_register()". To address this issue, I think it's
reasonable to move creation of wiphy debugfs directory during
allocation of wiphy device as per following change (untested though):

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 341402b..239e9ff 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -473,6 +473,10 @@ struct wiphy *wiphy_new_nm(const struct
cfg80211_ops *ops, int sizeof_priv,
                }
        }

+       /* add to debugfs */
+       rdev->wiphy.debugfsdir = debugfs_create_dir(wiphy_name(&rdev->wiphy),
+                                                   ieee80211_debugfs_dir);
+
        INIT_LIST_HEAD(&rdev->wiphy.wdev_list);
        INIT_LIST_HEAD(&rdev->beacon_registrations);
        spin_lock_init(&rdev->beacon_registrations_lock);
@@ -903,10 +907,6 @@ int wiphy_register(struct wiphy *wiphy)
        list_add_rcu(&rdev->list, &cfg80211_rdev_list);
        cfg80211_rdev_list_generation++;

-       /* add to debugfs */
-       rdev->wiphy.debugfsdir = debugfs_create_dir(wiphy_name(&rdev->wiphy),
-                                                   ieee80211_debugfs_dir);
-
        cfg80211_debugfs_rdev_add(rdev);
        nl80211_notify_wiphy(rdev, NL80211_CMD_NEW_WIPHY);

With this change we can remove this hidden debugfs dependency. Please
give it a try and let me know if it works for you.

-Sumit

>
> This patch was backported to multiple stable kernel versions:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.4.y&id=a8ce3412e8a22279e1bdc81c3c2bacd3785c1577
>
> Hauke