[U-Boot] [PATCH] usb: eth: fix memalign() parameter order

From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/usb/eth/usb_ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c index b9c9a8402e39..36734e2e51b3 100644 --- a/drivers/usb/eth/usb_ether.c +++ b/drivers/usb/eth/usb_ether.c @@ -73,7 +73,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize) }
ueth->rxsize = rxsize; - ueth->rxbuf = memalign(rxsize, ARCH_DMA_MINALIGN); + ueth->rxbuf = memalign(ARCH_DMA_MINALIGN, rxsize); if (!ueth->rxbuf) return -ENOMEM;

On 02/12/2016 09:56 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Oh this is a nice catch!
Acked-by: Marek Vasut marex@denx.de
drivers/usb/eth/usb_ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c index b9c9a8402e39..36734e2e51b3 100644 --- a/drivers/usb/eth/usb_ether.c +++ b/drivers/usb/eth/usb_ether.c @@ -73,7 +73,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize) }
ueth->rxsize = rxsize;
- ueth->rxbuf = memalign(rxsize, ARCH_DMA_MINALIGN);
- ueth->rxbuf = memalign(ARCH_DMA_MINALIGN, rxsize); if (!ueth->rxbuf) return -ENOMEM;

On 12 February 2016 at 13:57, Marek Vasut marex@denx.de wrote:
On 02/12/2016 09:56 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Oh this is a nice catch!
Acked-by: Marek Vasut marex@denx.de
drivers/usb/eth/usb_ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c index b9c9a8402e39..36734e2e51b3 100644 --- a/drivers/usb/eth/usb_ether.c +++ b/drivers/usb/eth/usb_ether.c @@ -73,7 +73,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize) }
ueth->rxsize = rxsize;
ueth->rxbuf = memalign(rxsize, ARCH_DMA_MINALIGN);
ueth->rxbuf = memalign(ARCH_DMA_MINALIGN, rxsize); if (!ueth->rxbuf) return -ENOMEM;
Oh dear. For some reason I thought that was fixed. But I see from this thread that it did not fix the problem at the time and I didn't send a patch either:
http://lists.denx.de/pipermail/u-boot/2015-August/222440.html
Reviewed-by: Simon Glass sjg@chromium.org
Thanks Stephen for finding this and actually fixing it :-)
- Simon

On Fri, Feb 12, 2016 at 2:56 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Yikes.
Reviewed-by: Joe Hershberger joe.hershberger@ni.com

On 02/12/2016 10:15 PM, Joe Hershberger wrote:
On Fri, Feb 12, 2016 at 2:56 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Yikes.
Reviewed-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot-usb/master, thanks.

On 02/12/2016 02:16 PM, Marek Vasut wrote:
On 02/12/2016 10:15 PM, Joe Hershberger wrote:
On Fri, Feb 12, 2016 at 2:56 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Yikes.
Reviewed-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot-usb/master, thanks.
Thanks. Any chance of a pull request into mainline? This issue is causing lots of crashing on one of the systems in my test setup, which potentially masks other problems.

On 02/22/2016 05:56 PM, Stephen Warren wrote:
On 02/12/2016 02:16 PM, Marek Vasut wrote:
On 02/12/2016 10:15 PM, Joe Hershberger wrote:
On Fri, Feb 12, 2016 at 2:56 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The alignment and size were swapped, leading to malloc heap corruption.
On my system, this sometimes caused U-Boot to crash during or after certain USB Ethernet operations.
Fixes: c8c2797c3810 ("dm: usb: eth: Support driver model with USB Ethernet") Signed-off-by: Stephen Warren swarren@nvidia.com
Yikes.
Reviewed-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot-usb/master, thanks.
Thanks. Any chance of a pull request into mainline? This issue is causing lots of crashing on one of the systems in my test setup, which potentially masks other problems.
Done, sorry for the delay.
participants (4)
-
Joe Hershberger
-
Marek Vasut
-
Simon Glass
-
Stephen Warren