diff mbox

[U-Boot] Patch: Fix device enumeration through API.

Message ID 318C7EDF-BBD2-4C66-ACE7-7171BA76B34B@freebsd.org
State Superseded
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Tim Kientzle Feb. 22, 2012, 6:34 a.m. UTC 
The one-line patch below fixes device enumeration through the
U-Boot API.

Device enumeration crashes when the system in question doesn't
have any RAM mapped to address zero (I discovered this on a
BeagleBone board), since the enumeration calls get_dev with a
NULL ifname sometimes which then gets passed down to strncmp().

This fix simply ensures that get_dev returns NULL when invoked
with a NULL ifname.

This could also be fixed by reworking the device enumeration to
never call get_dev with a NULL argument, but that's a much more
extensive change.  (get_dev is called from several places and the
code is driven by a list that's constructed in a way that naturally
leaves lots of NULLs.)

Cheers,

Tim Kientzle

Comments

Anatolij Gustschin March 26, 2012, 11:06 a.m. UTC | #1 
Hi Tim,

Thanks for the patch. Please see some comments below.

On Tue, 21 Feb 2012 22:34:35 -0800
Tim Kientzle <kientzle@freebsd.org> wrote:

> The one-line patch below fixes device enumeration through the
> U-Boot API.
> 
> Device enumeration crashes when the system in question doesn't
> have any RAM mapped to address zero (I discovered this on a
> BeagleBone board), since the enumeration calls get_dev with a
> NULL ifname sometimes which then gets passed down to strncmp().
> 
> This fix simply ensures that get_dev returns NULL when invoked
> with a NULL ifname.
> 
> This could also be fixed by reworking the device enumeration to
> never call get_dev with a NULL argument, but that's a much more
> extensive change.  (get_dev is called from several places and the
> code is driven by a list that's constructed in a way that naturally
> leaves lots of NULLs.)
> 
> Cheers,
> 
> Tim Kientzle
> 
> diff --git a/disk/part.c b/disk/part.c
> index f07a17f..1a82539 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -84,7 +84,7 @@ block_dev_desc_t *get_dev(char* ifname, int dev)
>  #ifdef CONFIG_NEEDS_MANUAL_RELOC
>  	name += gd->reloc_off;
>  #endif
> -	while (drvr->name) {
> +	while (ifname && drvr->name) {
>  		name = drvr->name;
>  		reloc_get_dev = drvr->get_dev;

I would prefer just checking for ifname == NULL at the top
of the function and not on each loop iteration. Also please
add your Signed-off-by when submitting patches.

Thanks,
Anatolij
diff mbox

Patch

diff --git a/disk/part.c b/disk/part.c
index f07a17f..1a82539 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -84,7 +84,7 @@  block_dev_desc_t *get_dev(char* ifname, int dev)
 #ifdef CONFIG_NEEDS_MANUAL_RELOC
 	name += gd->reloc_off;
 #endif
-	while (drvr->name) {
+	while (ifname && drvr->name) {
 		name = drvr->name;
 		reloc_get_dev = drvr->get_dev;
 #ifdef CONFIG_NEEDS_MANUAL_RELOC