envlist_unsetenv() looked up the entry to remove with
strncmp(entry->ev_var, env, strlen(env)). The comparison length is
the requested name's length, so any stored entry whose name *starts*
with that name compares equal. envlist_setenv() inserts at the head
of the list, so the first hit wins: with FOO=... stored first and
FOOBAR=... stored afterward, envlist_unsetenv("FOO") iterates from
the head, matches FOOBAR=... on the prefix, and drops it instead of
FOO=...
linux-user and bsd-user reach this code via the -U command-line
switch, so the bug is reachable from a normal qemu-user invocation.
envlist_setenv() used the same strncmp pattern but with
envname_len = (eq_sign - env + 1), so the '=' byte sat inside the
compared window and acted as an implicit boundary. setenv was
therefore not buggy -- but the safety lived in the byte layout of
ev_var rather than in the entry, so a future edit could easily
drift the two sites apart again.
Store the name length on each entry at insertion time and compare
with explicit length equality plus memcmp via a small helper. Use
the helper at both lookup sites so the boundary becomes a
structural property of the entry: envlist_unsetenv() stops
prefix-matching, and envlist_setenv()'s self-search no longer
depends on the '=' byte serving as a sentinel.
Fixes: 04a6dfebb6b5 ("linux-user: Add generic env variable handling")
Signed-off-by: Denis V. Lunev <[email protected]>
Cc: Stefan Hajnoczi <[email protected]>
Cc: Markus Armbruster <[email protected]>
Cc: Paolo Bonzini <[email protected]>
---
util/envlist.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/util/envlist.c b/util/envlist.c
index 15fdbb109d..196c92c190 100644
--- a/util/envlist.c
+++ b/util/envlist.c
@@ -3,7 +3,8 @@
#include "qemu/envlist.h"
struct envlist_entry {
- const char *ev_var; /* actual env value */
+ const char *ev_var; /* actual env value: "NAME=VALUE" */
+ size_t ev_name_len; /* length of NAME (offset of '=') */
QLIST_ENTRY(envlist_entry) ev_link;
};
@@ -12,6 +13,13 @@ struct envlist {
size_t el_count; /* number of entries */
};
+static inline bool envlist_name_eq(const struct envlist_entry *entry,
+ const char *name, size_t name_len)
+{
+ return entry->ev_name_len == name_len &&
+ memcmp(entry->ev_var, name, name_len) == 0;
+}
+
/*
* Allocates new envlist and returns pointer to it.
*/
@@ -67,7 +75,7 @@ envlist_setenv(envlist_t *envlist, const char *env)
/* find out first equals sign in given env */
if ((eq_sign = strchr(env, '=')) == NULL)
return (EINVAL);
- envname_len = eq_sign - env + 1;
+ envname_len = eq_sign - env;
/*
* If there already exists variable with given name
@@ -76,8 +84,9 @@ envlist_setenv(envlist_t *envlist, const char *env)
*/
for (entry = envlist->el_entries.lh_first; entry != NULL;
entry = entry->ev_link.le_next) {
- if (strncmp(entry->ev_var, env, envname_len) == 0)
+ if (envlist_name_eq(entry, env, envname_len)) {
break;
+ }
}
if (entry != NULL) {
@@ -90,6 +99,7 @@ envlist_setenv(envlist_t *envlist, const char *env)
entry = g_malloc(sizeof(*entry));
entry->ev_var = g_strdup(env);
+ entry->ev_name_len = envname_len;
QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
return (0);
@@ -119,8 +129,9 @@ envlist_unsetenv(envlist_t *envlist, const char *env)
envname_len = strlen(env);
for (entry = envlist->el_entries.lh_first; entry != NULL;
entry = entry->ev_link.le_next) {
- if (strncmp(entry->ev_var, env, envname_len) == 0)
+ if (envlist_name_eq(entry, env, envname_len)) {
break;
+ }
}
if (entry != NULL) {
QLIST_REMOVE(entry, ev_link);
--
2.51.0
