[U-Boot] [PATCH] env: don't add an empty key to the env hashtable

[U-Boot] [PATCH] env: don't add an empty key to the env hashtable

[Lucian Cojocar]

If the environment contains an entry like "=value" "\0" we should skip
this key/value. Otherwise, U-Boot will enter in an infinite loop.

Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
---
 lib/hashtable.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/hashtable.c b/lib/hashtable.c
index 07ebfb2..8f5a6f8 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
 		*sp++ = '\0';	/* terminate value */
 		++dp;
 
+		/* skip this entry if the name is empty */
+		if (*name == 0)
+			continue;
+
 		/* Skip variables which are not supposed to be processed */
 		if (!drop_var_from_set(name, nvars, localvars))
 			continue;
-- 
1.7.9.5

[U-Boot] [PATCH] env: don't add an empty key to the env hashtable

[Wolfgang Denk]

Dear Lucian Cojocar,

In message <1367099787-25602-1-git-send-email-cojocar@gmail.com> you wrote:
> If the environment contains an entry like "=value" "\0" we should skip
> this key/value. Otherwise, U-Boot will enter in an infinite loop.
> 
> Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
> ---
>  lib/hashtable.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 07ebfb2..8f5a6f8 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
>  		*sp++ = '\0';	/* terminate value */
>  		++dp;
>  
> +		/* skip this entry if the name is empty */
> +		if (*name == 0)
> +			continue;

NAK.   This would be a serious error, and silently ignoring this is
downright wrong.

If such a situation should ever happen, it must cause a fatal error.

Could you please explain which exact problem you are trying to fix? 
I have to admit that I cannot think of a usage szenario that would
lead to such an error.

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
You cannot propel yourself forward by patting yourself on the back.

[U-Boot] [PATCH] env: don't add an empty key to the env hashtable

[Lucian Cojocar]

On 04/28/2013 12:35 AM, Wolfgang Denk wrote:
> Dear Lucian Cojocar,
>
> In message <1367099787-25602-1-git-send-email-cojocar@gmail.com> you wrote:
>> If the environment contains an entry like "=value" "\0" we should skip
>> this key/value. Otherwise, U-Boot will enter in an infinite loop.
>>
>> Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
>> ---
>>   lib/hashtable.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/hashtable.c b/lib/hashtable.c
>> index 07ebfb2..8f5a6f8 100644
>> --- a/lib/hashtable.c
>> +++ b/lib/hashtable.c
>> @@ -870,6 +870,10 @@ int himport_r(struct hsearch_data *htab,
>>   		*sp++ = '\0';	/* terminate value */
>>   		++dp;
>>
>> +		/* skip this entry if the name is empty */
>> +		if (*name == 0)
>> +			continue;
>
> NAK.   This would be a serious error, and silently ignoring this is
> downright wrong.
>
> If such a situation should ever happen, it must cause a fatal error.
>

I agree.

> Could you please explain which exact problem you are trying to fix?
> I have to admit that I cannot think of a usage szenario that would
> lead to such an error.

I had an error (typo) in my predefined environment. Basically I had this:

#define CONFIG_EXTRA_ENV_SETTINGS \
	"key" "\0" "=value" "\0"

It would be nice if U-Boot told me that there was something wrong with 
my environment rather than just hang.

Thanks,
Lucian

[U-Boot] [PATCH] env: don't add an empty key to the env hashtable

[Wolfgang Denk]

Dear Lucian Cojocar,

In message <517C5A5A.2010106@gmail.com> you wrote:
>
> I had an error (typo) in my predefined environment. Basically I had this:
> 
> #define CONFIG_EXTRA_ENV_SETTINGS \
> 	"key" "\0" "=value" "\0"
> 
> It would be nice if U-Boot told me that there was something wrong with 
> my environment rather than just hang.

Agreed - it should throw an error.

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
We have found all life forms in the galaxy are  capable  of  superior
development.
	-- Kirk, "The Gamesters of Triskelion", stardate 3211.7

[U-Boot] [PATCH v2] env: throw an error when an empty key is used

[Lucian Cojocar]

If the environment contains an entry like "=value" "\0" we should throw
an error when parsing the environment. Otherwise, U-Boot will enter in
an infinite loop.

Signed-off-by: Lucian Cojocar <cojocar@gmail.com>
---
Changes for v2:
    - Throw an error instead of silently skip the empty key

 lib/hashtable.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/hashtable.c b/lib/hashtable.c
index 07ebfb2..779580b 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -870,6 +870,12 @@ int himport_r(struct hsearch_data *htab,
 		*sp++ = '\0';	/* terminate value */
 		++dp;
 
+		if (*name == 0) {
+			debug("INSERT: unable to use an empty key\n");
+			__set_errno(EINVAL);
+			return 0;
+		}
+
 		/* Skip variables which are not supposed to be processed */
 		if (!drop_var_from_set(name, nvars, localvars))
 			continue;
-- 
1.7.9.5

[U-Boot] [U-Boot, v2] env: throw an error when an empty key is used

[Tom Rini]

On Sun, Apr 28, 2013 at 11:31:57AM -0000, Lucian Cojocar wrote:

> If the environment contains an entry like "=value" "\0" we should throw
> an error when parsing the environment. Otherwise, U-Boot will enter in
> an infinite loop.
> 
> Signed-off-by: Lucian Cojocar <cojocar@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130510/0d21b8ac/attachment.pgp>