From: "Michal Vokáč" <michal.vokac@ysoft.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym_pause
Date: Thu, 26 Sep 2019 10:11:34 +0200 [thread overview]
Message-ID: <385ff818-b852-b542-fc64-f4b884060f71@ysoft.com> (raw)
In-Reply-To: <e7bffa36-f218-d71e-c416-38aff73d35dd@ysoft.com>
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@hydraco:~# 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@hydraco:~# 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@hydraco:~# 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")
next prev parent reply other threads:[~2019-09-26 8:11 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-24 11:27 [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym_pause Michal Vokáč
2019-09-24 12:31 ` Andrew Lunn
2019-09-24 13:10 ` Michal Vokáč
2019-09-25 0:44 ` Andrew Lunn
2019-09-25 7:06 ` Michal Vokáč
2019-09-26 8:11 ` Michal Vokáč [this message]
2019-09-26 6:12 ` Heiner Kallweit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=385ff818-b852-b542-fc64-f4b884060f71@ysoft.com \
--to=michal.vokac@ysoft.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).