[U-Boot] [PATCH] Fix memory stomper in DFU. Loop NULL-initted one past allocated array size.
From: Michael Cashwell mboards@prograde.net
The memory layout arranged itself such that a long-standing memory stomper in a DFU prepare callback used during USB registration mangled the malloc heap enough to cause my board to panic much later in a call to free(). Since it hadn't happened before but was repeatable I decided to investigate before it vanished again.
The actual stomp happened in this line after the for loop: f_dfu->function[i] = NULL;
git blame says this code was introduced here: b819ddbf (Lukasz Majewski 2012-08-06 14:41:06 +0200 587)
I'm not sure if the function[] array actually needs a NULL entry at the end. If so then this patch is the right fix. If it really always knows the last array index and doesn't need the NULL then removing the offending assignment would be better. Not knowing makes this patch safer.
Signed-off-by: Michael Cashwell mboards@prograde.net --- drivers/usb/gadget/f_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index a322ae5..b24de09 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -589,7 +589,7 @@ static int dfu_prepare_function(struct f_dfu *f_dfu, int n) struct usb_interface_descriptor *d; int i = 0;
- f_dfu->function = calloc(sizeof(struct usb_descriptor_header *), n); + f_dfu->function = calloc(sizeof(struct usb_descriptor_header *), n + 1); if (!f_dfu->function) goto enomem;
On Fri, 12 Jul 2013 13:48:51 -0400 mboards@prograde.net wrote:
Hi Michael,
From: Michael Cashwell mboards@prograde.net
The memory layout arranged itself such that a long-standing memory stomper in a DFU prepare callback used during USB registration mangled the malloc heap enough to cause my board to panic much later in a call to free(). Since it hadn't happened before but was repeatable I decided to investigate before it vanished again.
The actual stomp happened in this line after the for loop: f_dfu->function[i] = NULL;
Thanks for investigation, but I've already fixed that:
dfu:function: Fix number of allocated DFU function pointers SHA1: e059a400ad780328cd5ad22c396298cac520c856
This patch has been included to v2013.07-rc2.
git blame says this code was introduced here: b819ddbf (Lukasz Majewski 2012-08-06 14:41:06 +0200 587)
I'm not sure if the function[] array actually needs a NULL entry at the end. If so then this patch is the right fix. If it really always knows the last array index and doesn't need the NULL then removing the offending assignment would be better. Not knowing makes this patch safer.
Signed-off-by: Michael Cashwell mboards@prograde.net
drivers/usb/gadget/f_dfu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index a322ae5..b24de09 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -589,7 +589,7 @@ static int dfu_prepare_function(struct f_dfu *f_dfu, int n) struct usb_interface_descriptor *d; int i = 0;
- f_dfu->function = calloc(sizeof(struct usb_descriptor_header
*), n);
- f_dfu->function = calloc(sizeof(struct usb_descriptor_header
*), n + 1); if (!f_dfu->function) goto enomem;
Anyway its nice to hear, that +1 user of DFU is out there :-)
Best regards, Lukasz Majewski
participants (2)
-
Lukasz Majewski -
mboards＠prograde.net