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

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

[Tim Kientzle]

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;
 #ifdef CONFIG_NEEDS_MANUAL_RELOC

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

[Anatolij Gustschin]

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

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

[Tim Kientzle]

Hello, Anatolij,

Thank you for your response.  Modified patch below:


On Mar 26, 2012, at 4:06 AM, Anatolij Gustschin wrote:

> 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.


Signed-off-by: Tim Kientzle <kientzle@freebsd.org>

diff --git a/disk/part.c b/disk/part.c
index f07a17f..35a2def 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -80,6 +80,9 @@ block_dev_desc_t *get_dev(char* ifname, int dev)
        block_dev_desc_t* (*reloc_get_dev)(int dev);
        char *name;
 
+       if (ifname == NULL)
+               return NULL;
+
        name = drvr->name;
 #ifdef CONFIG_NEEDS_MANUAL_RELOC
        name += gd->reloc_off;

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

[Wolfgang Denk]

Dear Tim Kientzle,

please do not top post / full quote.  Especially not when submitting
patches.

In message <2A2CB860-6BB6-46C5-B508-F7F97B1EB930@freebsd.org> you wrote:
> Hello, Anatolij,
> 
> Thank you for your response.  Modified patch below:
> 
> 
> On Mar 26, 2012, at 4:06 AM, Anatolij Gustschin wrote:
> 
> > 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
...

All this would become the commit message - this doesn't work.  Any
such comments belong to the comment section, i. e. below the "---" line 
which would bepresentif you generated your patches using "git format-
patch".


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If there was anything that depressed him more than his own  cynicism,
it was that quite often it still wasn't as cynical as real life.
                                 - Terry Pratchett, _Guards! Guards!_

[U-Boot] [PATCH v2] disk/part.c: Fix device enumeration through API

[Anatolij Gustschin]

From: Tim Kientzle <kientzle@freebsd.org>

The 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.

Signed-off-by: Tim Kientzle <kientzle@freebsd.org>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - resend with fixed whitespace errors and properly added
   commit log.

I've queued this patch in my staging branch. Thanks!

Anatolij

 disk/part.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index f07a17f..8ca5d4b 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -80,6 +80,9 @@ block_dev_desc_t *get_dev(char* ifname, int dev)
 	block_dev_desc_t* (*reloc_get_dev)(int dev);
 	char *name;
 
+	if (!ifname)
+		return NULL;
+
 	name = drvr->name;
 #ifdef CONFIG_NEEDS_MANUAL_RELOC
 	name += gd->reloc_off;
-- 
1.7.1