[PATCH] net: bnx2x: fix variable dereferenced before check

Subsystems: broadcom bnx2x 10 gigabit ethernet driver, networking drivers, the rest

STALE1755d

12 messages, 6 authors, 2021-11-18 · open the first message on its own page

[PATCH] net: bnx2x: fix variable dereferenced before check

From: Pavel Skripkin <hidden>
Date: 2021-11-13 22:43:25

Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.

Fixes: 523224a3b3cd ("bnx2x, cnic, bnx2i: use new FW/HSI")
Signed-off-by: Pavel Skripkin <redacted>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
index 1835d2e451c0..fc7fce642666 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
@@ -635,11 +635,13 @@ static int bnx2x_ilt_client_mem_op(struct bnx2x *bp, int cli_num,
 {
 	int i, rc;
 	struct bnx2x_ilt *ilt = BP_ILT(bp);
-	struct ilt_client_info *ilt_cli = &ilt->clients[cli_num];
+	struct ilt_client_info *ilt_cli;
 
 	if (!ilt || !ilt->lines)
 		return -1;
 
+	ilt_cli = &ilt->clients[cli_num];
+
 	if (ilt_cli->flags & (ILT_CLIENT_SKIP_INIT | ILT_CLIENT_SKIP_MEM))
 		return 0;
 
-- 
2.33.1

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: Pavel Skripkin <hidden>
Date: 2021-11-13 22:43:25

On 11/14/21 01:36, Pavel Skripkin wrote:
Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.

Fixes: 523224a3b3cd ("bnx2x, cnic, bnx2i: use new FW/HSI")
Signed-off-by: Pavel Skripkin <redacted>
---
  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
Btw, looks like GR-everest-linux-l2@marvell.com doesn't exist anymore. 
It's listed in 2 MAINTAINERS entries. Should it be removed from 
MAINTAINERS file?


Quoting private email from postmaster@marvel.com:
Delivery has failed to these recipients or groups:

gr-everest-linux-l2@marvell.com<mailto:gr-everest-linux-l2@marvell.com>
The email address you entered couldn't be found. Please check the recipient's email address and try to resend the message. If the problem continues, please contact your helpdesk.




With regards,
Pavel Skripkin

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-15 13:31:45

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller [off-list ref]:

On Sun, 14 Nov 2021 01:36:36 +0300 you wrote:
Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.

[...]
Here is the summary with links:
  - net: bnx2x: fix variable dereferenced before check
    https://git.kernel.org/netdev/net/c/f8885ac89ce3

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

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-11-15 14:13:21

On Sun, 14 Nov 2021 01:41:59 +0300 Pavel Skripkin wrote:
Btw, looks like GR-everest-linux-l2@marvell.com doesn't exist anymore. 
It's listed in 2 MAINTAINERS entries. Should it be removed from 
MAINTAINERS file?
Yes, if it bounces it should be removed. Can you send a patch?

[PATCH] MAINTAINERS: remove GR-everest-linux-l2@marvell.com

From: Pavel Skripkin <hidden>
Date: 2021-11-15 21:18:35

I've sent a patch to GR-everest-linux-l2@marvell.com few days ago and
got a reply from postmaster@marvell.com:

	Delivery has failed to these recipients or groups:

	gr-everest-linux-l2@marvell.com<mailto:gr-everest-linux-l2@marvell.com>
	The email address you entered couldn't be found. Please check the
	recipient's email address and try to resend the message. If the problem
	continues, please contact your helpdesk.

Since this email bounces, let's remove it from MAINTAINERS file.

Signed-off-by: Pavel Skripkin <redacted>
---

CCed others related maintainers from marvell, maybe they know what
happened with this email

---
 MAINTAINERS | 2 --
 1 file changed, 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2345ce8521..305008573765 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3733,7 +3733,6 @@ F:	drivers/scsi/bnx2i/
 BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
 M:	Ariel Elior <aelior@marvell.com>
 M:	Sudarsana Kalluru <skalluru@marvell.com>
-M:	GR-everest-linux-l2@marvell.com
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	drivers/net/ethernet/broadcom/bnx2x/
@@ -15593,7 +15592,6 @@ F:	drivers/scsi/qedi/
 
 QLOGIC QL4xxx ETHERNET DRIVER
 M:	Ariel Elior <aelior@marvell.com>
-M:	GR-everest-linux-l2@marvell.com
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	drivers/net/ethernet/qlogic/qed/
-- 
2.33.1

Re: [PATCH] MAINTAINERS: remove GR-everest-linux-l2@marvell.com

From: Alok Prasad <hidden>
Date: 2021-11-16 08:10:34

I've sent a patch to GR-everest-linux-l2@marvell.com few days ago and
got a reply from postmaster@marvell.com:

     Delivery has failed to these recipients or groups:

     gr-everest-linux-l2@marvell.com<mailto:gr-everest-linux-l2@marvell.com>
     The email address you entered couldn't be found. Please check the
     recipient's email address and try to resend the message. If the problem
     continues, please contact your helpdesk.

Since this email bounces, let's remove it from MAINTAINERS file.

Signed-off-by: Pavel Skripkin <redacted>
---

CCed others related maintainers from marvell, maybe they know what
happened with this email
Pavel,
Yes gr-everest-linux-l2@marvell.com has been disabled, But please
add Manish Chopra [off-list ref] to the maintainer list.

Thanks,
Alok
---

Re: [PATCH] MAINTAINERS: remove GR-everest-linux-l2@marvell.com

From: Alok Prasad <hidden>
Date: 2021-11-16 08:16:20

Pavel,
Yes gr-everest-linux-l2@marvell.com has been disabled, But please
add Manish Chopra [off-list ref] to the maintainer list.
Sorry , Correction it should Manish Chopra [off-list ref]
Thanks,
Alok

[PATCH v2] MAINTAINERS: remove GR-everest-linux-l2@marvell.com

From: Pavel Skripkin <hidden>
Date: 2021-11-16 14:13:34

I've sent a patch to GR-everest-linux-l2@marvell.com few days ago and
got a reply from postmaster@marvell.com:

	Delivery has failed to these recipients or groups:

	gr-everest-linux-l2@marvell.com<mailto:gr-everest-linux-l2@marvell.com>
	The email address you entered couldn't be found. Please check the
	recipient's email address and try to resend the message. If the problem
	continues, please contact your helpdesk.

As requested by Alok Prasad, replacing GR-everest-linux-l2@marvell.com
with Manish Chopra's email address. [0]

Link: https://lore.kernel.org/all/20211116081601.11208-1-palok@marvell.com/ [0]
Signed-off-by: Pavel Skripkin <redacted>
---

Changes in v2:
	Replaced GR-everest-linux-l2@marvell.com with Manish Chopra's email, as
	requested by Alok.

---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2345ce8521..10c8ae3a8c73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3733,7 +3733,7 @@ F:	drivers/scsi/bnx2i/
 BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
 M:	Ariel Elior <aelior@marvell.com>
 M:	Sudarsana Kalluru <skalluru@marvell.com>
-M:	GR-everest-linux-l2@marvell.com
+M:	Manish Chopra <manishc@marvell.com>
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	drivers/net/ethernet/broadcom/bnx2x/
@@ -15593,7 +15593,7 @@ F:	drivers/scsi/qedi/
 
 QLOGIC QL4xxx ETHERNET DRIVER
 M:	Ariel Elior <aelior@marvell.com>
-M:	GR-everest-linux-l2@marvell.com
+M:	Manish Chopra <manishc@marvell.com>
 L:	netdev@vger.kernel.org
 S:	Supported
 F:	drivers/net/ethernet/qlogic/qed/
-- 
2.33.1

Re: [PATCH v2] MAINTAINERS: remove GR-everest-linux-l2@marvell.com

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-17 03:10:13

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski [off-list ref]:

On Tue, 16 Nov 2021 17:13:03 +0300 you wrote:
I've sent a patch to GR-everest-linux-l2@marvell.com few days ago and
got a reply from postmaster@marvell.com:

	Delivery has failed to these recipients or groups:

	gr-everest-linux-l2@marvell.com<mailto:gr-everest-linux-l2@marvell.com>
	The email address you entered couldn't be found. Please check the
	recipient's email address and try to resend the message. If the problem
	continues, please contact your helpdesk.

[...]
Here is the summary with links:
  - [v2] MAINTAINERS: remove GR-everest-linux-l2@marvell.com
    https://git.kernel.org/netdev/net/c/0a83f96f8709

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

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: Johan Hovold <johan@kernel.org>
Date: 2021-11-18 08:53:00

[ Adding Dan. ]

On Sun, Nov 14, 2021 at 01:36:36AM +0300, Pavel Skripkin wrote:
Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.
It seems smatch is confused here. There is no dereference happening
until after the check, we're just determining the address when
initialising ilt_cli.

I know this has been applied, and the change itself is fine, but the
patch description is wrong and the Fixes tag is unwarranted.
 
quoted hunk
Fixes: 523224a3b3cd ("bnx2x, cnic, bnx2i: use new FW/HSI")
Signed-off-by: Pavel Skripkin <redacted>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
index 1835d2e451c0..fc7fce642666 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h
@@ -635,11 +635,13 @@ static int bnx2x_ilt_client_mem_op(struct bnx2x *bp, int cli_num,
 {
 	int i, rc;
 	struct bnx2x_ilt *ilt = BP_ILT(bp);
-	struct ilt_client_info *ilt_cli = &ilt->clients[cli_num];
+	struct ilt_client_info *ilt_cli;
 
 	if (!ilt || !ilt->lines)
 		return -1;
 
+	ilt_cli = &ilt->clients[cli_num];
+
 	if (ilt_cli->flags & (ILT_CLIENT_SKIP_INIT | ILT_CLIENT_SKIP_MEM))
 		return 0;
Johan

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: Pavel Skripkin <hidden>
Date: 2021-11-18 09:01:14

On 11/18/21 11:51, Johan Hovold wrote:
[ Adding Dan. ]

On Sun, Nov 14, 2021 at 01:36:36AM +0300, Pavel Skripkin wrote:
quoted
Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.
It seems smatch is confused here. There is no dereference happening
until after the check, we're just determining the address when
initialising ilt_cli.

I know this has been applied, and the change itself is fine, but the
patch description is wrong and the Fixes tag is unwarranted.
  
I agree. I came up with same thing after the patch has been applied. I 
thought about a revert, but seems it's not necessary, since there is no 
function change.

I should check smatch warnings more carefully next time, can't say why I 
didn't notice it before sending :(

thanks



With regards,
Pavel Skripkin

Re: [PATCH] net: bnx2x: fix variable dereferenced before check

From: Dan Carpenter <hidden>
Date: 2021-11-18 15:10:24

On Thu, Nov 18, 2021 at 09:51:57AM +0100, Johan Hovold wrote:
[ Adding Dan. ]

On Sun, Nov 14, 2021 at 01:36:36AM +0300, Pavel Skripkin wrote:
quoted
Smatch says:
	bnx2x_init_ops.h:640 bnx2x_ilt_client_mem_op()
	warn: variable dereferenced before check 'ilt' (see line 638)

Move ilt_cli variable initialization _after_ ilt validation, because
it's unsafe to deref the pointer before validation check.
It seems smatch is confused here. There is no dereference happening
until after the check, we're just determining the address when
initialising ilt_cli.
Yep.  You're right.  I will fix this.  I've written a fix and I'll test
it tonight.

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help