netdev - Re: [PATCH] net: lan78xx: fix "enabled interrupts" warninig

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: <20190417161357.4e29fedd@xhacker.debian>
Date:   Wed, 17 Apr 2019 08:22:07 +0000
From:   Jisheng Zhang <Jisheng.Zhang@...aptics.com>
To:     "RaghuramChary.Jallipalli@...rochip.com" 
        <RaghuramChary.Jallipalli@...rochip.com>
CC:     "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "marc.zyngier@....com" <marc.zyngier@....com>,
        "Woojung.Huh@...rochip.com" <Woojung.Huh@...rochip.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: lan78xx: fix "enabled interrupts" warninig

On Wed, 17 Apr 2019 03:49:46 +0000 <RaghuramChary.Jallipalli@...rochip.com> wrote:

> 
> Hi Jisheng,

Hi,

> 
> > > I want to understand if there is any functionality impact with this warning?  
> > Because I'm afraid if the current changes are removed we might hit some
> > other issues (or older ones). We have to go through rigorous testing before
> > going ahead.
> >
> > Warning indicates there's something wrong in the code.  
> 
> Agree that the code is incorrect. Just wanted to understand if you had any functionality impact too.
> 
> >
> > IMHO phy_mac_interrupt() and PHY_IGNORE_INTERRUPT is the correct
> > solution. If the phy_mac_interrupt() poll is fixed, I think maybe old issue
> > which commit cc89c323a30e want to fix won't exist.
> >  
> 
> I tried to reproduce the problem in PC environment but did not see the warnings.
> However, I tried your patch and did plug/unplug tests(rmmod/insmod continuously) and observed call traces from kernel. I don't see these traces without your patch.
> Attached log.

I believe this is another bug in lan78xx driver: after phy_disconnect() if
there's intr urb coming, we will call phy_mac_interrupt, so trigger
phy_state_machine(), but now the phydev->attached_dev is NULL, so NULL
pointer deference. Can you please try below patch:

--->8
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 246a0d1bbc6c..27d6fbdd58c1 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3467,6 +3467,8 @@ static void lan78xx_disconnect(struct usb_interface *intf)
 	net = dev->net;
 	phydev = net->phydev;
 
+	usb_kill_urb(dev->urb_intr);
+
 	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
 	phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0);
 
@@ -3483,7 +3485,6 @@ static void lan78xx_disconnect(struct usb_interface *intf)
 
 	lan78xx_unbind(dev, intf);
 
-	usb_kill_urb(dev->urb_intr);
 	usb_free_urb(dev->urb_intr);
 
 	free_netdev(net);