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

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

[Sergej Bauer]

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>
---
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 */
 

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

[Jakub Kicinski]

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!

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

[Sergej Bauer]

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.

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

[Sven Van Asbroeck]

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.

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

[Sergej Bauer]

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.

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

[Jakub Kicinski]

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. 

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

[Sergej Bauer]

This is the 4th 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"
command 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 instead of -EIO (review by Andrew Lunn)

v4:
Sven Van Asbruck noticed that the patch was being applied cleanly to the 5.9
branch, so the tag “Fixes” was added as Jakub suggested.

Signed-off-by: Sergej Bauer <sbauer@blackbox.su>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Fixes: 4d94282afd95 ("lan743x: Add power management support")
---
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 */
 

Re: [PATCH v4] lan743x: fix for potential NULL pointer dereference with bare card

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf.git (refs/heads/master):

On Tue, 15 Dec 2020 19:12:45 +0300 you wrote:
> This is the 4th 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"
> command 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
> ...
> 
> [...]

Here is the summary with links:
  - [v4] lan743x: fix for potential NULL pointer dereference with bare card
    https://git.kernel.org/bpf/bpf/c/e9e13b6adc33

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v4] lan743x: fix for potential NULL pointer dereference with bare card

[Jakub Kicinski]

On Tue, 15 Dec 2020 23:39:14 +0000 patchwork-bot+netdevbpf@kernel.org
wrote:
> Hello:
> 
> This patch was applied to bpf/bpf.git (refs/heads/master):
> 
> On Tue, 15 Dec 2020 19:12:45 +0300 you wrote:
> > This is the 4th 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"
> > command 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
> > ...
> > 
> > [...]  
> 
> Here is the summary with links:
>   - [v4] lan743x: fix for potential NULL pointer dereference with bare card
>     https://git.kernel.org/bpf/bpf/c/e9e13b6adc33
> 
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
> 

Heh the bot got confused, I think.

What I meant when I said "let's wait for the merge window" was that
the patch will not hit upstream until the merge window. It's now in
Linus's tree. I'll make a submission of stable patches to Greg at the
end of the week and I'll include this patch.

Thanks!

Re: [PATCH v4] lan743x: fix for potential NULL pointer dereference with bare card

[Sergej Bauer]

On Wednesday, December 16, 2020 4:12:42 AM MSK Jakub Kicinski wrote:
> On Tue, 15 Dec 2020 23:39:14 +0000 patchwork-bot+netdevbpf@kernel.org
> 
> wrote:
> > Hello:
> > 
> > This patch was applied to bpf/bpf.git (refs/heads/master):
> > 
> > On Tue, 15 Dec 2020 19:12:45 +0300 you wrote:
> > > This is the 4th 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" command where ethN is the interface with lan743x card.
> > > Example:
> > > 
> > > $ sudo ethtool eth7
> > > dmesg:
...
> > > [...]
> > 
> > Here is the summary with links:
> >   - [v4] lan743x: fix for potential NULL pointer dereference with bare
> >   card
> >   
> >     https://git.kernel.org/bpf/bpf/c/e9e13b6adc33
> > 
> > You are awesome, thank you!
> > --
> > Deet-doot-dot, I am a bot.
> > https://korg.docs.kernel.org/patchwork/pwbot.html
> 
> Heh the bot got confused, I think.
> 
> What I meant when I said "let's wait for the merge window" was that
> the patch will not hit upstream until the merge window. It's now in
> Linus's tree. I'll make a submission of stable patches to Greg at the
> end of the week and I'll include this patch.
> 
> Thanks!

I think, firstly the bot was confused by me :-\
I should have asked you what exactly did you mean with "let's wait for the 
merge window"...
That's completely my fault, sorry for that.

                                Regards,
                                        Sergej.