[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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.0

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Vadim Fedorenko]

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.

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

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

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

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”.

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

> 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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

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

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Jonathan Corbet]

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