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 14:31:26 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Michal Vokáč <michal.vokac@...ft.com>
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 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;
 
-       phy_support_asym_pause(phy);
+       if (phy)
+               phy_support_asym_pause(phy);
 
        return 0;
 }

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.

       Andrew