netdev - Re: [PATCH net] Revert "ravb: Fixed to be able to unload modules"

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: <32351a6c-07ea-2a5c-0edd-4ef92dc38002@gmail.com>
Date:   Tue, 22 Sep 2020 12:53:37 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...il.com>
To:     Geert Uytterhoeven <geert+renesas@...der.be>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Yuusuke Ashizuka <ashiduka@...itsu.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH net] Revert "ravb: Fixed to be able to unload modules"

On 22.09.2020 10:29, Geert Uytterhoeven wrote:

> This reverts commit 1838d6c62f57836639bd3d83e7855e0ee4f6defc.
> 
> This commit moved the ravb_mdio_init() call (and thus the
> of_mdiobus_register() call) from the ravb_probe() to the ravb_open()
> call.  This causes a regression during system resume (s2idle/s2ram), as
> new PHY devices cannot be bound while suspended.
> 
> During boot, the Micrel PHY is detected like this:
> 
>      Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=228)
>      ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> 
> During system suspend, (A) defer_all_probes is set to true, and (B)
> usermodehelper_disabled is set to UMH_DISABLED, to avoid drivers being
> probed while suspended.
> 
>    A. If CONFIG_MODULES=n, phy_device_register() calling device_add()
>       merely adds the device, but does not probe it yet, as
>       really_probe() returns early due to defer_all_probes being set:
> 
>         dpm_resume+0x128/0x4f8
> 	 device_resume+0xcc/0x1b0
> 	   dpm_run_callback+0x74/0x340
> 	     ravb_resume+0x190/0x1b8
> 	       ravb_open+0x84/0x770
> 		 of_mdiobus_register+0x1e0/0x468
> 		   of_mdiobus_register_phy+0x1b8/0x250
> 		     of_mdiobus_phy_device_register+0x178/0x1e8
> 		       phy_device_register+0x114/0x1b8
> 			 device_add+0x3d4/0x798
> 			   bus_probe_device+0x98/0xa0
> 			     device_initial_probe+0x10/0x18
> 			       __device_attach+0xe4/0x140
> 				 bus_for_each_drv+0x64/0xc8
> 				   __device_attach_driver+0xb8/0xe0
> 				     driver_probe_device.part.11+0xc4/0xd8
> 				       really_probe+0x32c/0x3b8
> 
>       Later, phy_attach_direct() notices no PHY driver has been bound,
>       and falls back to the Generic PHY, leading to degraded operation:
> 
>         Generic PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=POLL)
>         ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
> 
>    B. If CONFIG_MODULES=y, request_module() returns early with -EBUSY due
>       to UMH_DISABLED, and MDIO initialization fails completely:
> 
>         mdio_bus e6800000.ethernet-ffffffff:00: error -16 loading PHY driver module for ID 0x00221622
>         ravb e6800000.ethernet eth0: failed to initialize MDIO
>         PM: dpm_run_callback(): ravb_resume+0x0/0x1b8 returns -16
>         PM: Device e6800000.ethernet failed to resume: error -16
> 
>       Ignoring -EBUSY in phy_request_driver_module(), like was done for
>       -ENOENT in commit 21e194425abd65b5 ("net: phy: fix issue with loading
>       PHY driver w/o initramfs"), would makes it fall back to the Generic
>       PHY, like in the CONFIG_MODULES=n case.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> Cc: stable@...r.kernel.org

Reviewed-by: Sergei Shtylyov <sergei.shtylyov@...il.com>

> ---
> Commit 1838d6c62f578366 ("ravb: Fixed to be able to unload modules") was
> already backported to stable v4.4, v4.9, v4.14, v4.19, v5.4, and v5.8),
> and thus needs to be reverted there, too.

    Ugh!
    Sorry for not noticing the issue with the original patch... :-/

MBR, Sergei