[PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Michael Ferolito]

The current behaviour of this function will dereference a null pointer
if the serial# environment variable is unset. This was discovered on a
board where U-Boot did not have access to the first 256MB of ram,
resulting in a board crash.
In the event that U-Boot has full access to memory, it will still read
from address 0, which is probably not optimal.
This simple check is enough to fix it

Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Heiko Schocher <hs@denx.de>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/g_dnl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
index 631969b340..f2540eb6de 100644
--- a/drivers/usb/gadget/g_dnl.c
+++ b/drivers/usb/gadget/g_dnl.c
@@ -207,7 +207,8 @@ void g_dnl_clear_detach(void)
 static int on_serialno(const char *name, const char *value, enum env_op op,
 		int flags)
 {
-	g_dnl_set_serialnumber((char *)value);
+	if (value)
+		g_dnl_set_serialnumber((char *)value);
 	return 0;
 }
 U_BOOT_ENV_CALLBACK(serialno, on_serialno);
-- 
2.48.1

Re: [PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Marek Vasut]

On 1/28/25 1:30 AM, Michael Ferolito wrote:
> The current behaviour of this function will dereference a null pointer
> if the serial# environment variable is unset. This was discovered on a
> board where U-Boot did not have access to the first 256MB of ram,
> resulting in a board crash.
> In the event that U-Boot has full access to memory, it will still read
> from address 0, which is probably not optimal.
> This simple check is enough to fix it
> 
> Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
The subject tag is "usb: gadget: g_dnl:" instead of "USB:" , with that 
fixed:

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks

Re: [PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Mattijs Korpershoek]

Hi Michael,

Thank you for the patch.

On mar., janv. 28, 2025 at 02:49, Marek Vasut <marex@denx.de> wrote:

> On 1/28/25 1:30 AM, Michael Ferolito wrote:
>> The current behaviour of this function will dereference a null pointer
>> if the serial# environment variable is unset. This was discovered on a
>> board where U-Boot did not have access to the first 256MB of ram,
>> resulting in a board crash.
>> In the event that U-Boot has full access to memory, it will still read
>> from address 0, which is probably not optimal.
>> This simple check is enough to fix it
>> 
>> Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> The subject tag is "usb: gadget: g_dnl:" instead of "USB:" , with that 
> fixed:
>
> Reviewed-by: Marek Vasut <marex@denx.de>

Agreed with Marek on this. Please add:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

As well.

Marek, since this is gadget, can this go through my tree?
In that case, Michael, I can fix up the subject when applying (to avoid
you sending a v2)

Let me know if that works.

Thanks,
Mattijs


>
> Thanks

Re: [PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Heiko Schocher]

Hello Mattijs,

On 28.01.25 10:11, Mattijs Korpershoek wrote:
> Hi Michael,
> 
> Thank you for the patch.
> 
> On mar., janv. 28, 2025 at 02:49, Marek Vasut <marex@denx.de> wrote:
> 
>> On 1/28/25 1:30 AM, Michael Ferolito wrote:
>>> The current behaviour of this function will dereference a null pointer
>>> if the serial# environment variable is unset. This was discovered on a
>>> board where U-Boot did not have access to the first 256MB of ram,
>>> resulting in a board crash.
>>> In the event that U-Boot has full access to memory, it will still read
>>> from address 0, which is probably not optimal.
>>> This simple check is enough to fix it
>>>
>>> Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> The subject tag is "usb: gadget: g_dnl:" instead of "USB:" , with that
>> fixed:
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
> 
> Agreed with Marek on this. Please add:
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 
> As well.
> 
> Marek, since this is gadget, can this go through my tree?
> In that case, Michael, I can fix up the subject when applying (to avoid
> you sending a v2)

There is already a v4, see:

https://lists.denx.de/pipermail/u-boot/2025-January/578746.html

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Mattijs Korpershoek]

Hi Heiko,

On mar., janv. 28, 2025 at 10:17, Heiko Schocher <hs@denx.de> wrote:

> Hello Mattijs,
>
> On 28.01.25 10:11, Mattijs Korpershoek wrote:
>> Hi Michael,
>> 
>> Thank you for the patch.
>> 
>> On mar., janv. 28, 2025 at 02:49, Marek Vasut <marex@denx.de> wrote:
>> 
>>> On 1/28/25 1:30 AM, Michael Ferolito wrote:
>>>> The current behaviour of this function will dereference a null pointer
>>>> if the serial# environment variable is unset. This was discovered on a
>>>> board where U-Boot did not have access to the first 256MB of ram,
>>>> resulting in a board crash.
>>>> In the event that U-Boot has full access to memory, it will still read
>>>> from address 0, which is probably not optimal.
>>>> This simple check is enough to fix it
>>>>
>>>> Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: Heiko Schocher <hs@denx.de>
>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>> The subject tag is "usb: gadget: g_dnl:" instead of "USB:" , with that
>>> fixed:
>>>
>>> Reviewed-by: Marek Vasut <marex@denx.de>
>> 
>> Agreed with Marek on this. Please add:
>> 
>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> 
>> As well.
>> 
>> Marek, since this is gadget, can this go through my tree?
>> In that case, Michael, I can fix up the subject when applying (to avoid
>> you sending a v2)
>
> There is already a v4, see:
>
> https://lists.denx.de/pipermail/u-boot/2025-January/578746.html

Indeed. Seems I'm a bit slow today :)

Thanks for sharing the link.

>
> bye,
> Heiko
> -- 
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH v2] USB: Fix NULLPTR dereference when serial# is unset

[Marek Vasut]

On 1/28/25 10:11 AM, Mattijs Korpershoek wrote:
> Hi Michael,
> 
> Thank you for the patch.
> 
> On mar., janv. 28, 2025 at 02:49, Marek Vasut <marex@denx.de> wrote:
> 
>> On 1/28/25 1:30 AM, Michael Ferolito wrote:
>>> The current behaviour of this function will dereference a null pointer
>>> if the serial# environment variable is unset. This was discovered on a
>>> board where U-Boot did not have access to the first 256MB of ram,
>>> resulting in a board crash.
>>> In the event that U-Boot has full access to memory, it will still read
>>> from address 0, which is probably not optimal.
>>> This simple check is enough to fix it
>>>
>>> Signed-off-by: Michael Ferolito <michaelsunn101@gmail.com>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> The subject tag is "usb: gadget: g_dnl:" instead of "USB:" , with that
>> fixed:
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
> 
> Agreed with Marek on this. Please add:
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 
> As well.
> 
> Marek, since this is gadget, can this go through my tree?
Yes of course.