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

Date:   Thu, 26 Sep 2019 10:11:34 +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 15:10, Michal Vokáč wrote:
> 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...

I found and fixed the problem with CPU and USER ports not working.

The issue was in the fact that the qca8k driver incorrectly allocates
the dsa_switch structure with DSA_MAX_PORTS (which is 12) but the QCA8K
switches support up 7 ports.

So the dsa_tree_setup_switches() looped with num_ports set to 12.
Since commit [1] disables all unused ports, ports 7-11 were disabled.
What happened is that some registers were incorrectly rewritten by
the qca8k_port_disable() calls.

I will submit a patch very soon.

Thank you,
Michal

[1] 0394a63acfe2 ("net: dsa: enable and disable all ports")