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

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;

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

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

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

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;

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!
participants (3)
-
Lucian Cojocar
-
Tom Rini
-
Wolfgang Denk