* [PATCH] net: bnx2x: fix variable dereferenced before check
@ 2021-11-13 22:36 Pavel Skripkin
2021-11-13 22:41 ` Pavel Skripkin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-11-13 22:36 UTC (permalink / raw)
To: aelior, skalluru, GR-everest-linux-l2, davem, kuba
Cc: netdev, linux-kernel, Pavel Skripkin
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 <paskripkin@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-13 22:36 [PATCH] net: bnx2x: fix variable dereferenced before check Pavel Skripkin
@ 2021-11-13 22:41 ` Pavel Skripkin
2021-11-15 14:12 ` Jakub Kicinski
2021-11-15 13:30 ` patchwork-bot+netdevbpf
2021-11-18 8:51 ` Johan Hovold
2 siblings, 1 reply; 7+ messages in thread
From: Pavel Skripkin @ 2021-11-13 22:41 UTC (permalink / raw)
To: aelior, skalluru, davem, kuba; +Cc: netdev, linux-kernel
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 <paskripkin@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-13 22:36 [PATCH] net: bnx2x: fix variable dereferenced before check Pavel Skripkin
2021-11-13 22:41 ` Pavel Skripkin
@ 2021-11-15 13:30 ` patchwork-bot+netdevbpf
2021-11-18 8:51 ` Johan Hovold
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-15 13:30 UTC (permalink / raw)
To: Pavel Skripkin
Cc: aelior, skalluru, GR-everest-linux-l2, davem, kuba, netdev, linux-kernel
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-13 22:41 ` Pavel Skripkin
@ 2021-11-15 14:12 ` Jakub Kicinski
0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-11-15 14:12 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: aelior, skalluru, davem, netdev, linux-kernel
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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-13 22:36 [PATCH] net: bnx2x: fix variable dereferenced before check Pavel Skripkin
2021-11-13 22:41 ` Pavel Skripkin
2021-11-15 13:30 ` patchwork-bot+netdevbpf
@ 2021-11-18 8:51 ` Johan Hovold
2021-11-18 9:00 ` Pavel Skripkin
2021-11-18 15:09 ` Dan Carpenter
2 siblings, 2 replies; 7+ messages in thread
From: Johan Hovold @ 2021-11-18 8:51 UTC (permalink / raw)
To: Pavel Skripkin, Dan Carpenter
Cc: aelior, skalluru, GR-everest-linux-l2, davem, kuba, netdev, linux-kernel
[ 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.
> Fixes: 523224a3b3cd ("bnx2x, cnic, bnx2i: use new FW/HSI")
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-18 8:51 ` Johan Hovold
@ 2021-11-18 9:00 ` Pavel Skripkin
2021-11-18 15:09 ` Dan Carpenter
1 sibling, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-11-18 9:00 UTC (permalink / raw)
To: Johan Hovold, Dan Carpenter
Cc: aelior, skalluru, GR-everest-linux-l2, davem, kuba, netdev, linux-kernel
On 11/18/21 11:51, Johan Hovold wrote:
> [ 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.
>
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: bnx2x: fix variable dereferenced before check
2021-11-18 8:51 ` Johan Hovold
2021-11-18 9:00 ` Pavel Skripkin
@ 2021-11-18 15:09 ` Dan Carpenter
1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2021-11-18 15:09 UTC (permalink / raw)
To: Johan Hovold
Cc: Pavel Skripkin, aelior, skalluru, GR-everest-linux-l2, davem,
kuba, netdev, linux-kernel
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:
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-18 15:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-13 22:36 [PATCH] net: bnx2x: fix variable dereferenced before check Pavel Skripkin
2021-11-13 22:41 ` Pavel Skripkin
2021-11-15 14:12 ` Jakub Kicinski
2021-11-15 13:30 ` patchwork-bot+netdevbpf
2021-11-18 8:51 ` Johan Hovold
2021-11-18 9:00 ` Pavel Skripkin
2021-11-18 15:09 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).