drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 12 Jul 2026 20:25:10 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “rnpgbe_rm_adapter”.
Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
Pass the required address directly to a function call.
This issue was detected by using the Coccinelle software.
Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..93cf757c951c 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -245,7 +245,6 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
static void rnpgbe_rm_adapter(struct pci_dev *pdev)
{
struct mucse *mucse = pci_get_drvdata(pdev);
- struct mucse_hw *hw = &mucse->hw;
struct net_device *netdev;
int err;
@@ -253,7 +252,7 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
return;
netdev = mucse->netdev;
unregister_netdev(netdev);
- err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
+ err = rnpgbe_send_notify(&mucse->hw, false, mucse_fw_powerup);
if (err)
dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
free_netdev(netdev);
--
2.55.0On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
>
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.
Please can you show the path taken that mucse is actually NULL.
I tend to agree, that the correct change is to remove the NULL pointer
test.
Andrew
---
pw-bot: cr
>> The address of a data structure member was determined before >> a corresponding null pointer check in the implementation of >> the function “rnpgbe_rm_adapter”. > > Please can you show the path taken that mucse is actually NULL. The applied sanity check indicates a corresponding possibility. Which result representations would you find helpful for the requested data flow analysis? > I tend to agree, that the correct change is to remove the NULL pointer > test. Will the omission of such a check gain wider acceptance? Regards, Markus
On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
>
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.
>
> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> Pass the required address directly to a function call.
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
There is no NULL dereference here. It's just pointer math.
No need for a Fixes tag.
regards,
dan carpenter
>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
>>
>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>> Pass the required address directly to a function call.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
>
> There is no NULL dereference here. It's just pointer math.
> No need for a Fixes tag.
How does your view fit to information in an article like “Fun with NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
https://lwn.net/Articles/342330/
Regards,
Markus
On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “rnpgbe_rm_adapter”.
> >>
> >> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >> Pass the required address directly to a function call.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> >
> > There is no NULL dereference here. It's just pointer math.
> > No need for a Fixes tag.
>
> How does your view fit to information in an article like “Fun with NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
> https://lwn.net/Articles/342330/
>
Of course you can't have NULL pointer dereferences but this is not a
dereference, it's just pointer math.
regards,
dan carpenter
>>>> The address of a data structure member was determined before
>>>> a corresponding null pointer check in the implementation of
>>>> the function “rnpgbe_rm_adapter”.
>>>>
>>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>>>> Pass the required address directly to a function call.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>>>
>>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
>>>
>>> There is no NULL dereference here. It's just pointer math.
>>> No need for a Fixes tag.
>>
>> How does your view fit to information in an article like “Fun with NULL pointers, part 1”
>> (by Jonathan Corbet from 2009-07-20)?>> https://lwn.net/Articles/342330/
>>
>
> Of course you can't have NULL pointer dereferences but this is not a
> dereference, it's just pointer math.
Does your understanding of programming language details differ from the view
by Jonathan Corbet?
Regards,
Markus
Markus Elfring <Markus.Elfring@web.de> writes: > Does your understanding of programming language details differ from the view > by Jonathan Corbet? I'm not sure why you keep dragging me into this - I have not opined on this particular situation, which is definitely *not* a dereference. jon
>> Does your understanding of programming language details differ from the view >> by Jonathan Corbet? > > I'm not sure why you keep dragging me into this You are one of the contributors who pointed a specific control flow out as a bug. > - I have not opined on > this particular situation, I got an other impression. > which is definitely *not* a dereference. Does such a feedback indicate a questionable communication difficulty? Regards, Markus
On Thu, Jul 16, 2026 at 10:20:42AM +0200, Markus Elfring wrote:
> >>>> The address of a data structure member was determined before
> >>>> a corresponding null pointer check in the implementation of
> >>>> the function “rnpgbe_rm_adapter”.
> >>>>
> >>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >>>> Pass the required address directly to a function call.
> >>>>
> >>>> This issue was detected by using the Coccinelle software.
> >>>>
> >>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> >>>
> >>> There is no NULL dereference here. It's just pointer math.
> >>> No need for a Fixes tag.
> >>
> >> How does your view fit to information in an article like “Fun with NULL pointers, part 1”
> >> (by Jonathan Corbet from 2009-07-20)?>> https://lwn.net/Articles/342330/
> >>
> >
> > Of course you can't have NULL pointer dereferences but this is not a
> > dereference, it's just pointer math.
> Does your understanding of programming language details differ from the view
> by Jonathan Corbet?
Jonathan Corbet's article talks about dereferences.
p = tun->sk;
^^^^^^^
This is a dereference.
p = &tun->sk;
^
This is pointer math. It's not a dereference.
regards,
dan carpenter
> Jonathan Corbet's article talks about dereferences. > > p = tun->sk; > ^^^^^^^ > This is a dereference. > > p = &tun->sk; > ^ > This is pointer math. It's not a dereference. Can such a development view be confusing? Is the operator “address of” applied only after a pointer dereference attempt in this case? https://en.cppreference.com/c/language/operator_member_access Regards, Markus
On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “rnpgbe_rm_adapter”.
> >>
> >> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >> Pass the required address directly to a function call.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> >
> > There is no NULL dereference here. It's just pointer math.
> > No need for a Fixes tag.
>
> How does your view fit to information in an article like “Fun with
> NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
> https://lwn.net/Articles/342330/
It does fit, because the problematic code discussed in Jonathan Corbet's
article is of the type:
int i = ptr->i;
if (!ptr)
do_something();
while here we have:
int *i = &ptr->i;
if (!ptr)
do_something();
which at least in my test[1] is relevantly different. Note, I didn't
study the C standard if the compiler is free to optimize out
do_something() also in the 2nd case, but at least today gcc doesn't.
Best regards
Uwe
[1] Me knowing about ARM assembly, that's what I checked:
$ cat test.c
#include <stdio.h>
#include <stdlib.h>
struct mystruct {
int i;
char c;
};
int funcdirect(struct mystruct *ptr)
{
int i = ptr->i;
if (!ptr)
return -1;
printf("i = %d\n", i);
return 0;
}
int funcindirect(struct mystruct *ptr)
{
int *i = &ptr->i;
if (!ptr)
return -1;
printf("i = %d\n", *i);
return 0;
}
$ arm-linux-gnueabihf-gcc -O3 -c test.c
$ objdump -D test.o
test.o: file format elf32-littlearm
Disassembly of section .text:
00000000 <funcdirect>:
0: b508 push {r3, lr}
2: 4603 mov r3, r0
4: 4803 ldr r0, [pc, #12] @ (14 <funcdirect+0x14>)
6: 6819 ldr r1, [r3, #0]
8: 4478 add r0, pc
a: f7ff fffe bl 0 <printf>
e: 2000 movs r0, #0
10: bd08 pop {r3, pc}
12: bf00 nop
14: 00000008 andeq r0, r0, r8
00000018 <funcindirect>:
18: b138 cbz r0, 2a <funcindirect+0x12>
1a: 6801 ldr r1, [r0, #0]
1c: 4804 ldr r0, [pc, #16] @ (30 <funcindirect+0x18>)
1e: b508 push {r3, lr}
20: 4478 add r0, pc
22: f7ff fffe bl 0 <printf>
26: 2000 movs r0, #0
28: bd08 pop {r3, pc}
2a: f04f 30ff mov.w r0, #4294967295 @ 0xffffffff
2e: 4770 bx lr
30: 0000000c andeq r0, r0, ip
So in funcdirect the check is not present, while it is in funcindirect.
Am 15.07.26 um 15:12 schrieb Uwe Kleine-König:
> On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
>>>> The address of a data structure member was determined before
>>>> a corresponding null pointer check in the implementation of
>>>> the function “rnpgbe_rm_adapter”.
>>>>
>>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>>>> Pass the required address directly to a function call.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>>>
>>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
>>>
>>> There is no NULL dereference here. It's just pointer math.
>>> No need for a Fixes tag.
>>
>> How does your view fit to information in an article like “Fun with
>> NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
>> https://lwn.net/Articles/342330/
>
> It does fit, because the problematic code discussed in Jonathan Corbet's
> article is of the type:
>
> int i = ptr->i;
>
> if (!ptr)
> do_something();
>
> while here we have:
>
> int *i = &ptr->i;
>
> if (!ptr)
> do_something();
>
> which at least in my test[1] is relevantly different.
Jonathan Corbet provided the following information.
“…
But Herbert's patch added a line which dereferences the pointer prior to the check. That, of course, is a bug.
…”
> Note, I didn't
> study the C standard
I hope that clarification approaches can evolve further according to this information source.
> if the compiler is free to optimize out
> do_something() also in the 2nd case, but at least today gcc doesn't.
Can development interests grow also according to another clarification approach?
Does &((struct name *)NULL -> b) cause undefined behaviour in C11?
https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11
Would you prefer to omit a “sanity check” in the discussed function implementation?
Regards,
Markus
On Wed, Jul 15, 2026 at 08:45:48PM +0200, Markus Elfring wrote: > > Jonathan Corbet provided the following information. > > “… > But Herbert's patch added a line which dereferences the pointer prior to the check. That, of course, is a bug. > …” > Notice that it says "dereferences" not "does pointer math". > > > Note, I didn't > > study the C standard > > I hope that clarification approaches can evolve further according to this information source. > > > > if the compiler is free to optimize out > > do_something() also in the 2nd case, but at least today gcc doesn't. > > Can development interests grow also according to another clarification approach? > > Does &((struct name *)NULL -> b) cause undefined behaviour in C11? > https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11 > > > > Would you prefer to omit a “sanity check” in the discussed function implementation? > This is irrelevant. In C writing if (!p) and if (p == NULL) are always equivalent but weirdly the NULL doesn't have to zero. It's part of the C FAQ. https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that doesn't really matter unless you have a time machine. Even if you invented a time machine, the code here would still be fine because we have a NULL test before dereferencing the results of our pointer math. regards, dan carpenter
> In C writing if (!p) and if (p == NULL) are always equivalent but weirdly > the NULL doesn't have to zero. It's part of the C FAQ. > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that > doesn't really matter unless you have a time machine. Even if you invented > a time machine, the code here would still be fine because we have a NULL > test before dereferencing the results of our pointer math. Which expression evaluations would you interpret as pointer dereferences finally? https://c-faq.com/sx1/ Regards, Markus
On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote:
> > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> > the NULL doesn't have to zero. It's part of the C FAQ.
> > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> > doesn't really matter unless you have a time machine. Even if you invented
> > a time machine, the code here would still be fine because we have a NULL
> > test before dereferencing the results of our pointer math.
> Which expression evaluations would you interpret as pointer dereferences finally?
> https://c-faq.com/sx1/
This is all interesting, but nobody has yet explain how this function
can be called such that we have a NULL pointer.
Andrew
On Thu, Jul 16, 2026 at 03:24:22PM +0200, Andrew Lunn wrote: > On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote: > > > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly > > > the NULL doesn't have to zero. It's part of the C FAQ. > > > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that > > > doesn't really matter unless you have a time machine. Even if you invented > > > a time machine, the code here would still be fine because we have a NULL > > > test before dereferencing the results of our pointer math. > > Which expression evaluations would you interpret as pointer dereferences finally? > > https://c-faq.com/sx1/ > > This is all interesting, but nobody has yet explain how this function > can be called such that we have a NULL pointer. mucse cannot be NULL. We call pci_set_drvdata() during probe() and it is still valid when we call the remove() function. regards, dan carpenter
On Thu, Jul 16, 2026 at 04:34:05PM +0300, Dan Carpenter wrote:
> On Thu, Jul 16, 2026 at 03:24:22PM +0200, Andrew Lunn wrote:
> > On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote:
> > > > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> > > > the NULL doesn't have to zero. It's part of the C FAQ.
> > > > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> > > > doesn't really matter unless you have a time machine. Even if you invented
> > > > a time machine, the code here would still be fine because we have a NULL
> > > > test before dereferencing the results of our pointer math.
> > > Which expression evaluations would you interpret as pointer dereferences finally?
> > > https://c-faq.com/sx1/
> >
> > This is all interesting, but nobody has yet explain how this function
> > can be called such that we have a NULL pointer.
>
> mucse cannot be NULL.
>
> We call pci_set_drvdata() during probe() and it is still valid when
> we call the remove() function.
Same as what i thought. So while this is an interesting discussion,
please could someone post the real fix, remove the NULL pointer check.
Andrew
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 16 Jul 2026 19:15:06 +0200
The check for the pointer “mucse” was finally identified as undesirable.
Thus remove it.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2:
Andrew Lunn requested the removal of a null pointer check.
drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..82a3fbfb3c0a 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -249,8 +249,6 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
struct net_device *netdev;
int err;
- if (!mucse)
- return;
netdev = mucse->netdev;
unregister_netdev(netdev);
err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
--
2.55.0On Thu, Jul 16, 2026 at 07:26:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 16 Jul 2026 19:15:06 +0200
>
> The check for the pointer “mucse” was finally identified as undesirable.
I'm not sure finally is appropriate. It was one of the first questions
i asked. Also, i expect anybody looking at a Coccinelle report to take
the time to understand the code and decide what the correct fix is. We
have too many developers doing the minimum to make the tool happy,
without actually thinking.
Please also include the reasoning behind this. The commit message is
all about "Why?" Why is it undesirable?
And lastly, please always start a new thread for a new version of the
patch.
Andrew
---
pw-bot: cr
>> The check for the pointer “mucse” was finally identified as undesirable. > I'm not sure finally is appropriate. It was one of the first questions > i asked. Also, i expect anybody looking at a Coccinelle report to take > the time to understand the code and decide what the correct fix is. We > have too many developers doing the minimum to make the tool happy, > without actually thinking. > > Please also include the reasoning behind this. The commit message is > all about "Why?" Why is it undesirable? Why could a redundant sanity check be tolerated for a while? Regards, Markus
On 12/07/2026 19:35, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sun, 12 Jul 2026 20:25:10 +0200 > > The address of a data structure member was determined before > a corresponding null pointer check in the implementation of > the function “rnpgbe_rm_adapter”. The null check in rnpgbe_rm_adapter() looks more like defensive programming, which got into the code unnoticed. pdev will not exist if netdev is not allocated, where mucse is netdev private data.
>> The address of a data structure member was determined before >> a corresponding null pointer check in the implementation of >> the function “rnpgbe_rm_adapter”. > > The null check in rnpgbe_rm_adapter() looks more like defensive > programming, This can be fine, can't it? > which got into the code unnoticed. Some patch reviews happened accordingly. > pdev will not exist if > netdev is not allocated, where mucse is netdev private data. Would you like to reconsider the relevance of another null pointer check? Regards, Markus
© 2016 - 2026 Red Hat, Inc.