On Tue, Jul 19, 2022 at 5:53 AM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Jul 18, 2022 at 10:50:38PM -0700, Tong Zhang wrote:
> > This driver creates 4 debug files under [devname] folder. The devname
> > could be wlan0 initially, however it could be renamed later to e.g.
> > enx00e04c00000. This will cause problem during debug file teardown since
> > it uses netdev->name which is no longer wlan0. To solve this problem,
> > add a notifier to handle device renaming.
> >
> > Reported-by: Zheyu Ma <zheyuma97@xxxxxxxxx>
> > Tested-by: Zheyu Ma <zheyuma97@xxxxxxxxx>
> > Signed-off-by: Tong Zhang <ztong0001@xxxxxxxxx>
> > ---
> > drivers/staging/rtl8192u/r8192U.h | 1 +
> > drivers/staging/rtl8192u/r8192U_core.c | 35 ++++++++++++++++++++++-
> > drivers/staging/rtl8192u/r8192U_debugfs.c | 13 +++++++++
> > 3 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U.h b/drivers/staging/rtl8192u/r8192U.h
> > index e8860bb2b607..ccce37b7e2ae 100644
> > --- a/drivers/staging/rtl8192u/r8192U.h
> > +++ b/drivers/staging/rtl8192u/r8192U.h
> > @@ -1122,4 +1122,5 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
> >
> > void rtl8192_debugfs_init(struct net_device *dev);
> > void rtl8192_debugfs_exit(struct net_device *dev);
> > +void rtl8192_debugfs_rename(struct net_device *dev);
> > #endif
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index ac3716550505..5cc78c5bd706 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -4606,6 +4606,30 @@ static void rtl8192_usb_disconnect(struct usb_interface *intf)
> > RT_TRACE(COMP_DOWN, "wlan driver removed\n");
> > }
> >
> > +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> > + void *data)
> > +{
> > + struct net_device *netdev = netdev_notifier_info_to_dev(data);
> > +
> > + if (netdev->netdev_ops != &rtl8192_netdev_ops)
> > + goto out;
> > +
> > + switch (event) {
> > + case NETDEV_CHANGENAME:
> > + rtl8192_debugfs_rename(netdev);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > +out:
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rtl8192_usb_netdev_notifier = {
> > + .notifier_call = rtl8192_usb_netdev_event,
> > +};
> > +
> > static int __init rtl8192_usb_module_init(void)
> > {
> > int ret;
> > @@ -4615,10 +4639,16 @@ static int __init rtl8192_usb_module_init(void)
> > RT_TRACE(COMP_INIT, "Initializing module");
> > RT_TRACE(COMP_INIT, "Wireless extensions version %d", WIRELESS_EXT);
> >
> > + ret = register_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> > + if (ret) {
> > + pr_err("register_netdevice_notifier failed %d\n", ret);
> > + return ret;
> > + }
> > +
> > ret = ieee80211_debug_init();
> > if (ret) {
> > pr_err("ieee80211_debug_init() failed %d\n", ret);
> > - return ret;
> > + goto unregister_notifier;
> > }
> >
> > ret = ieee80211_crypto_init();
> > @@ -4660,6 +4690,8 @@ static int __init rtl8192_usb_module_init(void)
> > ieee80211_crypto_deinit();
> > debug_exit:
> > ieee80211_debug_exit();
> > +unregister_notifier:
> > + unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> > return ret;
> > }
> >
> > @@ -4671,6 +4703,7 @@ static void __exit rtl8192_usb_module_exit(void)
> > ieee80211_crypto_tkip_exit();
> > ieee80211_crypto_deinit();
> > ieee80211_debug_exit();
> > + unregister_netdevice_notifier(&rtl8192_usb_netdev_notifier);
> > RT_TRACE(COMP_DOWN, "Exiting");
> > }
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U_debugfs.c b/drivers/staging/rtl8192u/r8192U_debugfs.c
> > index 5c9376e50889..c94f5dfac96b 100644
> > --- a/drivers/staging/rtl8192u/r8192U_debugfs.c
> > +++ b/drivers/staging/rtl8192u/r8192U_debugfs.c
> > @@ -173,3 +173,16 @@ void rtl8192_debugfs_exit(struct net_device *dev)
> > priv->debugfs_dir = NULL;
> > }
> >
> > +void rtl8192_debugfs_rename(struct net_device *dev)
> > +{
> > + struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
>
> No need to cast.
>
> > +
> > + if (!priv->debugfs_dir)
> > + return;
>
> Why does it matter?
>
> > +
> > + if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
>
> Ick, never poke around in a dentry from debugfs if you can help it. You
> know you are being renamed, why does it matter to check or not?
>
> > + return;
> > +
> > + debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> > + priv->debugfs_dir->d_parent, dev->name);
>
> Don't poke around in the dentry for the parent here either. That feels
> racy and wrong. Isn't there some other way to get the parent?
>
> Also you can look up the dentry for this, no need to have it saved, like
> I said in patch 2.
I think we need to save the old entry here otherwise we don't know the
original name since dev->name has already been overwritten.
> thanks,
>
> greg k-h
