netdev - Re: [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym

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]
Date:   Tue, 24 Sep 2019 15:10:44 +0200
From:   Michal Vokáč <michal.vokac@...ft.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [BUG] Unable to handle kernel NULL pointer dereference in
 phy_support_asym_pause

On 24. 09. 19 14:31, Andrew Lunn wrote:
> On Tue, Sep 24, 2019 at 01:27:24PM +0200, Michal Vokáč wrote:
>> Hi,
>>
>> just tried booting latest next-20190920 on our imx6dl-yapp4-hydra platform
>> with QCA8334 switch and got this:
>>
>> [    7.424620] [<806840e0>] (phy_support_asym_pause) from [<80686724>] (qca8k_port_enable+0x40/0x48)
>> [    7.436911] [<806866e4>] (qca8k_port_enable) from [<80a74134>] (dsa_port_enable+0x3c/0x6c)
>> [    7.448629]  r7:00000000 r6:e88a02cc r5:e812d090 r4:e812d090
>> [    7.457708] [<80a740f8>] (dsa_port_enable) from [<80a730bc>] (dsa_register_switch+0x798/0xacc)
>> [    7.469833]  r5:e812d0cc r4:e812d090
> 
> Hi Michal
> 
> Please could you add a printk to verify it is the CPU port, and that
> in qca8k_port_enable() phy is a NULL pointer.
> 
> I think the fix is going to look something like:
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 16f15c93a102..86c80a873e30 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -939,7 +939,8 @@ qca8k_port_enable(struct dsa_switch *ds, int port,
>          qca8k_port_set_status(priv, port, 1);
>          priv->port_sts[port].enabled = 1;

  	dev_info(priv->dev, "port: %d, phy: %d", port, (u32)phy);

>   
> -       phy_support_asym_pause(phy);
> +       if (phy)
> +               phy_support_asym_pause(phy);
>   
>          return 0;
>   }
> 

I added the printk and the above fix and can confirm that it is the CPU
port and the phy is NULL pointer:

[    6.976366] qca8k 2188000.ethernet-1:0a: Using legacy PHYLIB callbacks. Please migrate to PHYLINK!
[    6.992021] qca8k 2188000.ethernet-1:0a: port: 0, phy: 0
[    7.001323] qca8k 2188000.ethernet-1:0a eth2 (uninitialized): PHY [2188000.ethernet-1:01] driver [Generic PHY]
[    7.014221] qca8k 2188000.ethernet-1:0a eth2 (uninitialized): phy: setting supported 00,00000000,000062ef advertising 00,00000000,000062ef
[    7.030598] qca8k 2188000.ethernet-1:0a eth1 (uninitialized): PHY [2188000.ethernet-1:02] driver [Generic PHY]
[    7.043500] qca8k 2188000.ethernet-1:0a eth1 (uninitialized): phy: setting supported 00,00000000,000062ef advertising 00,00000000,000062ef
[    7.063335] DSA: tree 0 setup

Now the device boots but there is a problem with the CPU port configuration:

root@...raco:~# ifconfig eth0 up
[  255.256047] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
[  255.272449] fec 2188000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
[  255.286539] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
root@...raco:~# ifconfig eth1 up
[  268.350078] qca8k 2188000.ethernet-1:0a: port: 3, phy: -393143296
[  268.364442] qca8k 2188000.ethernet-1:0a eth1: configuring for phy/ link mode
[  268.375400] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//Unknown/Unknown adv=00,00000000,000062ef pause=10 link=0 an=1
[  268.393901] qca8k 2188000.ethernet-1:0a eth1: phy link up /1Gbps/Full
[  268.404849] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//1Gbps/Full adv=00,00000000,00000000 pause=0e link=1 an=0
[  268.420740] qca8k 2188000.ethernet-1:0a eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[  268.432995] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
root@...raco:~# udhcpc -i eth1
udhcpc (v1.23.2) started
Sending discover...
Sending discover...
Sending discover...

On v5.3.1 it works OK:

root@...raco:~# ifconfig eth0 up
[   29.665997] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
[   29.682075] Generic PHY fixed-0:00: PHY advertising (00,00000200,000022e0) more modes than genphy supports, some modes not advertised.
[   29.698284] fec 2188000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
[   29.709568] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
root@...raco:~# ifconfig eth1 up
[   33.699923] qca8k 2188000.ethernet-1:0a eth1: configuring for phy/ link mode
[   33.714466] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//Unknown/Unknown adv=00,00000000,000062ef pause=10 link=0 an=1
[   33.735356] 8021q: adding VLAN 0 to HW filter on device eth1
[   33.755671] qca8k 2188000.ethernet-1:0a eth1: phy link up /1Gbps/Full
[   33.766071] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//1Gbps/Full adv=00,00000000,00000000 pause=0e link=1 an=0
[   33.781987] qca8k 2188000.ethernet-1:0a eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[   33.794290] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
oot@...raco:~# udhcpc -i eth1
udhcpc (v1.23.2) started
Sending discover...
Sending select for 10.1.8.103...

> But i want to take a closer look at what priv->port_sts[port].enabled
> = 1; does. Also, if there are any other port_enable() functions which
> always assume a valid phy device.

Anything else I can do to help with this?

Thank you!
Michal