netdev - RE: [PATCH] lan78xx: Connect phy early

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] [day] [month] [year] [list]

Message-ID: <CE371C1263339941885964188A0225FA3838A8@CHN-SV-EXMX03.mchp-main.com>
Date:   Thu, 15 Mar 2018 07:36:29 +0000
From:   <Nisar.Sayed@...rochip.com>
To:     <agraf@...e.de>, <Woojung.Huh@...rochip.com>
CC:     <UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <tbogendoerfer@...e.de>, <phil@...pberrypi.org>
Subject: RE: [PATCH] lan78xx: Connect phy early

Hi Alexander,

Thanks for the patch.

> @@ -2575,13 +2571,7 @@ static int lan78xx_stop(struct net_device *net)
>  	if (timer_pending(&dev->stat_monitor))
>  		del_timer_sync(&dev->stat_monitor);
> 
> -	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
> -	phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
> -
>  	phy_stop(net->phydev);
> -	phy_disconnect(net->phydev);
> -
> -	net->phydev = NULL;
> 
>  	clear_bit(EVENT_DEV_OPEN, &dev->flags);
>  	netif_stop_queue(net);

Please do add valid "phydev"  check before phy_stop, since "phy_disconnect" should be called before "unregister_netdev"

+ if (net->phydev)
	phy_stop(net->phydev);

> @@ -3481,6 +3471,11 @@ static void lan78xx_disconnect(struct
> usb_interface *intf)
>  	net = dev->net;
>  	unregister_netdev(net);
> 
> +	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
> +	phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
> +
> +	phy_disconnect(net->phydev);
> +
>  	cancel_delayed_work_sync(&dev->wq);
> 
>  	usb_scuttle_anchored_urbs(&dev->deferred);

Please move "unregister_netdev" after "phy_disconnect", otherwise "phy_disconnect" will fail while we disconnect USB.

> @@ -3634,6 +3629,10 @@ static int lan78xx_probe(struct usb_interface
> *intf,
>  	pm_runtime_set_autosuspend_delay(&udev->dev,
>  					 DEFAULT_AUTOSUSPEND_DELAY);
> 
> +	ret = lan78xx_phy_init(dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
> 
>  out3:
> --
> 2.12.3

We should "goto out4" instead of "return" upon "lan78xx_phy_init" fail

+ out4:
+	unregister_netdev(netdev);

In addition to current changes, you might have to take care of the following.

In function "lan78xx_reset_resume"

-  lan78xx_phy_init(dev);
+ phy_start(dev->net->phydev);

Thanks,
Sd.Nisar