[PATCH] USB: Fix NULLPTR dereference when serial# is unset
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: Lukasz Majewski l.majewski@samsung.com 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..d56f9c485e 100644 --- a/drivers/usb/gadget/g_dnl.c +++ b/drivers/usb/gadget/g_dnl.c @@ -50,7 +50,8 @@ static const char manufacturer[] = CONFIG_USB_GADGET_MANUFACTURER; void g_dnl_set_serialnumber(char *s) { memset(g_dnl_serial, 0, MAX_STRING_SERIAL); - strncpy(g_dnl_serial, s, MAX_STRING_SERIAL - 1); + if (s) + strncpy(g_dnl_serial, s, MAX_STRING_SERIAL - 1); }
static struct usb_device_descriptor device_desc = {
On 1/27/25 10:07 PM, 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.
How does one trigger this problem ?
The problem can be reproduced with U-Boot's sandbox with the default config. Then, run the following commands at the shell: Hit any key to stop autoboot: 0 => env set serial# test => env default -f serial# at which point the program will crash.
The following change will show that the env default -f command causes the null pointer dereference
void g_dnl_set_serialnumber(char *s) { memset(g_dnl_serial, 0, MAX_STRING_SERIAL); + printf("Reading serial from address %p\n", s); strncpy(g_dnl_serial, s, MAX_STRING_SERIAL - 1); }
On Mon, Jan 27, 2025 at 3:58 PM Marek Vasut marex@denx.de wrote:
On 1/27/25 10:07 PM, 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.
How does one trigger this problem ?
On 1/28/25 12:12 AM, Michael wrote:
The problem can be reproduced with U-Boot's sandbox with the default config. Then, run the following commands at the shell: Hit any key to stop autoboot: 0 => env set serial# test => env default -f serial# at which point the program will crash.
Maybe the fix needs to be one level higher ?
diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c index 631969b3405..f2540eb6ded 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);
participants (3)
-
Marek Vasut -
Michael -
Michael Ferolito