diff mbox series

[v3] lan743x: fix for potential NULL pointer dereference with bare card

Message ID 20201101223556.16116-1-sbauer@blackbox.su
State Superseded
Headers show
Series [v3] lan743x: fix for potential NULL pointer dereference with bare card | expand

Commit Message

Sergej Bauer Nov. 1, 2020, 10:35 p.m. UTC 
This is the 3rd revision of the patch fix for potential null pointer dereference
with lan743x card.

The simpliest way to reproduce: boot with bare lan743x and issue "ethtool ethN"
commant where ethN is the interface with lan743x card. Example:

$ sudo ethtool eth7
dmesg:
[  103.510336] BUG: kernel NULL pointer dereference, address: 0000000000000340
...
[  103.510836] RIP: 0010:phy_ethtool_get_wol+0x5/0x30 [libphy]
...
[  103.511629] Call Trace:
[  103.511666]  lan743x_ethtool_get_wol+0x21/0x40 [lan743x]
[  103.511724]  dev_ethtool+0x1507/0x29d0
[  103.511769]  ? avc_has_extended_perms+0x17f/0x440
[  103.511820]  ? tomoyo_init_request_info+0x84/0x90
[  103.511870]  ? tomoyo_path_number_perm+0x68/0x1e0
[  103.511919]  ? tty_insert_flip_string_fixed_flag+0x82/0xe0
[  103.511973]  ? inet_ioctl+0x187/0x1d0
[  103.512016]  dev_ioctl+0xb5/0x560
[  103.512055]  sock_do_ioctl+0xa0/0x140
[  103.512098]  sock_ioctl+0x2cb/0x3c0
[  103.512139]  __x64_sys_ioctl+0x84/0xc0
[  103.512183]  do_syscall_64+0x33/0x80
[  103.512224]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  103.512274] RIP: 0033:0x7f54a9cba427
...

Previous versions can be found at:
v1:
initial version
    https://lkml.org/lkml/2020/10/28/921

v2:
do not return from lan743x_ethtool_set_wol if netdev->phydev == NULL, just skip
the call of phy_ethtool_set_wol() instead.
    https://lkml.org/lkml/2020/10/31/380

v3:
in function lan743x_ethtool_set_wol:
use ternary operator instead of if-else sentence (review by Markus Elfring)
return -ENETDOWN insted of -EIO (review by Andrew Lunn)

Signed-off-by: Sergej Bauer <sbauer@blackbox.su>
---

Comments

Jakub Kicinski Nov. 4, 2020, 1:38 a.m. UTC | #1 
On Mon,  2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:
> This is the 3rd revision of the patch fix for potential null pointer dereference

> with lan743x card.

> 

> The simpliest way to reproduce: boot with bare lan743x and issue "ethtool ethN"

> commant where ethN is the interface with lan743x card. Example:

> 

> $ sudo ethtool eth7

> dmesg:

> [  103.510336] BUG: kernel NULL pointer dereference, address: 0000000000000340

> ...

> [  103.510836] RIP: 0010:phy_ethtool_get_wol+0x5/0x30 [libphy]

> ...

> [  103.511629] Call Trace:

> [  103.511666]  lan743x_ethtool_get_wol+0x21/0x40 [lan743x]

> [  103.511724]  dev_ethtool+0x1507/0x29d0

> [  103.511769]  ? avc_has_extended_perms+0x17f/0x440

> [  103.511820]  ? tomoyo_init_request_info+0x84/0x90

> [  103.511870]  ? tomoyo_path_number_perm+0x68/0x1e0

> [  103.511919]  ? tty_insert_flip_string_fixed_flag+0x82/0xe0

> [  103.511973]  ? inet_ioctl+0x187/0x1d0

> [  103.512016]  dev_ioctl+0xb5/0x560

> [  103.512055]  sock_do_ioctl+0xa0/0x140

> [  103.512098]  sock_ioctl+0x2cb/0x3c0

> [  103.512139]  __x64_sys_ioctl+0x84/0xc0

> [  103.512183]  do_syscall_64+0x33/0x80

> [  103.512224]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

> [  103.512274] RIP: 0033:0x7f54a9cba427


Applied, thanks!
Sergej Bauer Nov. 4, 2020, 7:46 p.m. UTC | #2 
On Wednesday, November 4, 2020 4:38:33 AM MSK Jakub Kicinski wrote:
> On Mon,  2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:
> > This is the 3rd revision of the patch fix for potential null pointer
> > dereference with lan743x card.
> > 
> > The simpliest way to reproduce: boot with bare lan743x and issue "ethtool
> > ethN" commant where ethN is the interface with lan743x card. Example:
> > 
> > $ sudo ethtool eth7
> > dmesg:
> > [  103.510336] BUG: kernel NULL pointer dereference, address:
> > 0000000000000340 ...
> > [  103.510836] RIP: 0010:phy_ethtool_get_wol+0x5/0x30 [libphy]
> > ...
> > [  103.511629] Call Trace:
> > [  103.511666]  lan743x_ethtool_get_wol+0x21/0x40 [lan743x]
> > [  103.511724]  dev_ethtool+0x1507/0x29d0
> > [  103.511769]  ? avc_has_extended_perms+0x17f/0x440
> > [  103.511820]  ? tomoyo_init_request_info+0x84/0x90
> > [  103.511870]  ? tomoyo_path_number_perm+0x68/0x1e0
> > [  103.511919]  ? tty_insert_flip_string_fixed_flag+0x82/0xe0
> > [  103.511973]  ? inet_ioctl+0x187/0x1d0
> > [  103.512016]  dev_ioctl+0xb5/0x560
> > [  103.512055]  sock_do_ioctl+0xa0/0x140
> > [  103.512098]  sock_ioctl+0x2cb/0x3c0
> > [  103.512139]  __x64_sys_ioctl+0x84/0xc0
> > [  103.512183]  do_syscall_64+0x33/0x80
> > [  103.512224]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [  103.512274] RIP: 0033:0x7f54a9cba427
> 
> Applied, thanks!

Hi, Jakub!
Thank you for taking the time to review my patch

                Regards,
                        Sergej.
Sven Van Asbroeck Nov. 26, 2020, 2:22 p.m. UTC | #3 
Hi Jakub, Sergej,

On Tue, Nov 3, 2020 at 8:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
>

> On Mon,  2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:

> > This is the 3rd revision of the patch fix for potential null pointer dereference

> > with lan743x card.

>

> Applied, thanks!


I noticed that this went into net-next. Is there a chance that it could also be
added to net? It's a real issue on 5.9, and the patch applies cleanly there.
Sergej Bauer Nov. 27, 2020, 8:39 a.m. UTC | #4 
Hi Sven.

> Hi Jakub, Sergej,

> 

> On Tue, Nov 3, 2020 at 8:41 PM Jakub Kicinski <kuba@kernel.org> wrote:

> > On Mon,  2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:

> > > This is the 3rd revision of the patch fix for potential null pointer

> > > dereference with lan743x card.

> > 

> > Applied, thanks!

> 

> I noticed that this went into net-next. Is there a chance that it could also

> be added to net? It's a real issue on 5.9, and the patch applies cleanly

> there.


It's my mistake - I missed pointing of stable branch.
Should I resend the patch (with tag Cc:stable@vger.kernel.org)?

                Regards,
                    Sergej.
Jakub Kicinski Nov. 27, 2020, 4:39 p.m. UTC | #5 
On Fri, 27 Nov 2020 11:39:41 +0300 Sergej Bauer wrote:
> > On Tue, Nov 3, 2020 at 8:41 PM Jakub Kicinski <kuba@kernel.org> wrote:  

> > > On Mon,  2 Nov 2020 01:35:55 +0300 Sergej Bauer wrote:  

> > > > This is the 3rd revision of the patch fix for potential null pointer

> > > > dereference with lan743x card.  

> > > 

> > > Applied, thanks!  

> > 

> > I noticed that this went into net-next. Is there a chance that it could also

> > be added to net? It's a real issue on 5.9, and the patch applies cleanly

> > there.  

> 

> It's my mistake - I missed pointing of stable branch.



> Should I resend the patch (with tag Cc:stable@vger.kernel.org)?


 Fixes tag would have been best. But it's too late
now, let's wait for the merge window and then we can back port it.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
index dcde496da7fb..c5de8f46cdd3 100644
--- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
+++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
@@ -780,7 +780,9 @@  static void lan743x_ethtool_get_wol(struct net_device *netdev,
 
 	wol->supported = 0;
 	wol->wolopts = 0;
-	phy_ethtool_get_wol(netdev->phydev, wol);
+
+	if (netdev->phydev)
+		phy_ethtool_get_wol(netdev->phydev, wol);
 
 	wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
 		WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
@@ -809,9 +811,8 @@  static int lan743x_ethtool_set_wol(struct net_device *netdev,
 
 	device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);
 
-	phy_ethtool_set_wol(netdev->phydev, wol);
-
-	return 0;
+	return netdev->phydev ? phy_ethtool_set_wol(netdev->phydev, wol)
+			: -ENETDOWN;
 }
 #endif /* CONFIG_PM */