* [PATCH 0/2] util/envlist: fix prefix-match in name lookup
@ 2026-05-20 21:26 Denis V. Lunev via qemu development
2026-05-20 21:26 ` [PATCH 1/2] util/envlist: fix prefix-match in envlist_unsetenv() " Denis V. Lunev via qemu development
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Denis V. Lunev via qemu development @ 2026-05-20 21:26 UTC (permalink / raw)
To: qemu-devel; +Cc: den, Stefan Hajnoczi, Markus Armbruster, Paolo Bonzini
A bug report against our downstream tree turned out to have its root
cause in plain mainstream code: envlist_unsetenv() does a prefix-match
lookup that drops the wrong entry when one stored name happens to be a
prefix of another. The downstream symptom is specific to our setup and
isn't interesting here -- the underlying lookup mistake is the part
worth fixing, and it is reachable from a normal qemu-user invocation
through the -U command-line option, so the fix belongs upstream.
Patch 1 fixes the lookup: each entry now stores its name length at
insertion time, and a tiny helper compares with explicit length
equality plus memcmp. envlist_setenv()'s self-search was accidentally
safe (it included the '=' byte in its strncmp window and that '='
served as a boundary) but is converted to the same helper so the name
boundary becomes a structural property of the entry rather than a
property of its byte layout. Without that, the two sites can easily
drift apart again.
Patch 2 backfils test coverage for util/envlist -- there was none --
in tests/unit/test-envlist. I verified that the regression case
(envlist_unsetenv("FOO") vs. a stored "FOOBAR=...") fails against the
pre-fix code and passes after the fix.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Denis V. Lunev (2):
util/envlist: fix prefix-match in envlist_unsetenv() name lookup
tests/unit: add test-envlist covering setenv/unsetenv name matching
tests/unit/meson.build | 1 +
tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++
util/envlist.c | 19 +++-
3 files changed, 212 insertions(+), 4 deletions(-)
create mode 100644 tests/unit/test-envlist.c
--
2.51.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] util/envlist: fix prefix-match in envlist_unsetenv() name lookup 2026-05-20 21:26 [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev via qemu development @ 2026-05-20 21:26 ` Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 2/2] tests/unit: add test-envlist covering setenv/unsetenv name matching Denis V. Lunev via qemu development 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev 2 siblings, 0 replies; 7+ messages in thread From: Denis V. Lunev via qemu development @ 2026-05-20 21:26 UTC (permalink / raw) To: qemu-devel; +Cc: den, Stefan Hajnoczi, Markus Armbruster, Paolo Bonzini 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 <den@openvz.org> Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] tests/unit: add test-envlist covering setenv/unsetenv name matching 2026-05-20 21:26 [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 1/2] util/envlist: fix prefix-match in envlist_unsetenv() " Denis V. Lunev via qemu development @ 2026-05-20 21:26 ` Denis V. Lunev via qemu development 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev 2 siblings, 0 replies; 7+ messages in thread From: Denis V. Lunev via qemu development @ 2026-05-20 21:26 UTC (permalink / raw) To: qemu-devel; +Cc: den, Stefan Hajnoczi, Markus Armbruster, Paolo Bonzini util/envlist had no test coverage. Add tests/unit/test-envlist exercising the public envlist API and pinning down the prefix-match hazard fixed in the previous commit: - envlist_unsetenv("FOO") must not remove an entry named "FOOBAR"; - envlist_setenv("FOO=...") must not replace an existing "FOOBAR=..." entry placed earlier in the list (envlist_setenv() inserts at the head, so the first prefix match wins under the old strncmp rule). Also cover the rest of the contract: head-insertion order observed through envlist_to_environ(), replacement of an existing variable, the count argument of envlist_to_environ(), and the documented EINVAL paths (NULL inputs, setenv without '=', unsetenv with '='). Signed-off-by: Denis V. Lunev <den@openvz.org> Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> --- tests/unit/meson.build | 1 + tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 tests/unit/test-envlist.c diff --git a/tests/unit/meson.build b/tests/unit/meson.build index de64f9501f..01cc540a45 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -49,6 +49,7 @@ tests = { 'test-qapi-util': [], 'test-interval-tree': [], 'test-fifo': [], + 'test-envlist': [], } if have_system or have_tools diff --git a/tests/unit/test-envlist.c b/tests/unit/test-envlist.c new file mode 100644 index 0000000000..53813dd4de --- /dev/null +++ b/tests/unit/test-envlist.c @@ -0,0 +1,196 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * envlist tests + * + * Copyright 2026 Virtuozzo International GmbH + * + * Authors: + * Denis V. Lunev <den@openvz.org> + */ + +#include "qemu/osdep.h" +#include "qemu/envlist.h" + +static void free_environ(char **env) +{ + char **p; + + for (p = env; *p != NULL; p++) { + g_free(*p); + } + g_free(env); +} + +static const char *find_env(char **env, const char *name) +{ + size_t name_len = strlen(name); + char **p; + + for (p = env; *p != NULL; p++) { + if (strncmp(*p, name, name_len) == 0 && (*p)[name_len] == '=') { + return *p + name_len + 1; + } + } + return NULL; +} + +static void test_envlist_basic(void) +{ + envlist_t *el = envlist_create(); + char **env; + size_t count; + + /* empty list */ + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 0); + g_assert_null(env[0]); + free_environ(env); + + /* add */ + g_assert_cmpint(envlist_setenv(el, "A=1"), ==, 0); + g_assert_cmpint(envlist_setenv(el, "B=2"), ==, 0); + + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 2); + g_assert_cmpstr(find_env(env, "A"), ==, "1"); + g_assert_cmpstr(find_env(env, "B"), ==, "2"); + free_environ(env); + + /* replace */ + g_assert_cmpint(envlist_setenv(el, "A=42"), ==, 0); + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 2); + g_assert_cmpstr(find_env(env, "A"), ==, "42"); + g_assert_cmpstr(find_env(env, "B"), ==, "2"); + free_environ(env); + + /* unset existing */ + g_assert_cmpint(envlist_unsetenv(el, "A"), ==, 0); + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 1); + g_assert_null(find_env(env, "A")); + g_assert_cmpstr(find_env(env, "B"), ==, "2"); + free_environ(env); + + /* unset non-existing is a no-op success */ + g_assert_cmpint(envlist_unsetenv(el, "NOPE"), ==, 0); + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 1); + free_environ(env); + + envlist_free(el); +} + +/* + * envlist_setenv() inserts at the head; envlist_to_environ() walks + * head-to-tail, so the last setenv comes out first. + */ +static void test_envlist_head_insertion_order(void) +{ + envlist_t *el = envlist_create(); + char **env; + size_t count; + + g_assert_cmpint(envlist_setenv(el, "A=1"), ==, 0); + g_assert_cmpint(envlist_setenv(el, "B=2"), ==, 0); + g_assert_cmpint(envlist_setenv(el, "C=3"), ==, 0); + + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 3); + g_assert_cmpstr(env[0], ==, "C=3"); + g_assert_cmpstr(env[1], ==, "B=2"); + g_assert_cmpstr(env[2], ==, "A=1"); + g_assert_null(env[3]); + + free_environ(env); + envlist_free(el); +} + +static void test_envlist_einval(void) +{ + envlist_t *el = envlist_create(); + + /* NULL list */ + g_assert_cmpint(envlist_setenv(NULL, "A=1"), ==, EINVAL); + g_assert_cmpint(envlist_unsetenv(NULL, "A"), ==, EINVAL); + + /* NULL string */ + g_assert_cmpint(envlist_setenv(el, NULL), ==, EINVAL); + g_assert_cmpint(envlist_unsetenv(el, NULL), ==, EINVAL); + + /* setenv: missing '=' */ + g_assert_cmpint(envlist_setenv(el, "NOEQ"), ==, EINVAL); + + /* unsetenv: name must not contain '=' */ + g_assert_cmpint(envlist_unsetenv(el, "A=B"), ==, EINVAL); + + envlist_free(el); +} + +/* + * Regression: envlist_unsetenv("FOO") must not remove an entry named + * "FOOBAR" -- the previous strncmp(entry, name, strlen(name)) lookup + * prefix-matched. To trigger the bug, the longer-named entry has to + * be ahead of the target in the list: envlist_setenv() inserts at + * the head, so we add FOO first and FOOBAR last. + */ +static void test_envlist_unsetenv_no_prefix_match(void) +{ + envlist_t *el = envlist_create(); + char **env; + size_t count; + + g_assert_cmpint(envlist_setenv(el, "FOO=y"), ==, 0); + g_assert_cmpint(envlist_setenv(el, "FOOBAR=x"), ==, 0); + + g_assert_cmpint(envlist_unsetenv(el, "FOO"), ==, 0); + + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 1); + g_assert_cmpstr(find_env(env, "FOOBAR"), ==, "x"); + g_assert_null(find_env(env, "FOO")); + + free_environ(env); + envlist_free(el); +} + +/* + * envlist_setenv() must not replace a prior FOOBAR=... entry when + * setting FOO=... The pre-fix code happened to be safe here only + * because it included the trailing '=' byte in its strncmp length; + * this test pins down the post-fix contract that the name boundary + * is a property of the entry, not of the encoded form. + */ +static void test_envlist_setenv_no_prefix_match(void) +{ + envlist_t *el = envlist_create(); + char **env; + size_t count; + + g_assert_cmpint(envlist_setenv(el, "FOOBAR=x"), ==, 0); + g_assert_cmpint(envlist_setenv(el, "FOO=y"), ==, 0); + + env = envlist_to_environ(el, &count); + g_assert_cmpuint(count, ==, 2); + g_assert_cmpstr(find_env(env, "FOOBAR"), ==, "x"); + g_assert_cmpstr(find_env(env, "FOO"), ==, "y"); + + free_environ(env); + envlist_free(el); +} + +int main(int argc, char *argv[]) +{ + g_test_init(&argc, &argv, NULL); + + g_test_add_func("/envlist/basic", test_envlist_basic); + g_test_add_func("/envlist/head_insertion_order", + test_envlist_head_insertion_order); + g_test_add_func("/envlist/einval", test_envlist_einval); + g_test_add_func("/envlist/unsetenv_no_prefix_match", + test_envlist_unsetenv_no_prefix_match); + g_test_add_func("/envlist/setenv_no_prefix_match", + test_envlist_setenv_no_prefix_match); + + return g_test_run(); +} -- 2.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup 2026-05-20 21:26 [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 1/2] util/envlist: fix prefix-match in envlist_unsetenv() " Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 2/2] tests/unit: add test-envlist covering setenv/unsetenv name matching Denis V. Lunev via qemu development @ 2026-05-28 9:24 ` Denis V. Lunev 2026-06-01 19:12 ` Stefan Hajnoczi ` (2 more replies) 2 siblings, 3 replies; 7+ messages in thread From: Denis V. Lunev @ 2026-05-28 9:24 UTC (permalink / raw) To: Denis V. Lunev, qemu-devel Cc: Stefan Hajnoczi, Markus Armbruster, Paolo Bonzini On 5/20/26 23:26, Denis V. Lunev wrote: > A bug report against our downstream tree turned out to have its root > cause in plain mainstream code: envlist_unsetenv() does a prefix-match > lookup that drops the wrong entry when one stored name happens to be a > prefix of another. The downstream symptom is specific to our setup and > isn't interesting here -- the underlying lookup mistake is the part > worth fixing, and it is reachable from a normal qemu-user invocation > through the -U command-line option, so the fix belongs upstream. > > Patch 1 fixes the lookup: each entry now stores its name length at > insertion time, and a tiny helper compares with explicit length > equality plus memcmp. envlist_setenv()'s self-search was accidentally > safe (it included the '=' byte in its strncmp window and that '=' > served as a boundary) but is converted to the same helper so the name > boundary becomes a structural property of the entry rather than a > property of its byte layout. Without that, the two sites can easily > drift apart again. > > Patch 2 backfils test coverage for util/envlist -- there was none -- > in tests/unit/test-envlist. I verified that the regression case > (envlist_unsetenv("FOO") vs. a stored "FOOBAR=...") fails against the > pre-fix code and passes after the fix. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Denis V. Lunev (2): > util/envlist: fix prefix-match in envlist_unsetenv() name lookup > tests/unit: add test-envlist covering setenv/unsetenv name matching > > tests/unit/meson.build | 1 + > tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++ > util/envlist.c | 19 +++- > 3 files changed, 212 insertions(+), 4 deletions(-) > create mode 100644 tests/unit/test-envlist.c > ping ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev @ 2026-06-01 19:12 ` Stefan Hajnoczi 2026-06-01 19:18 ` Stefan Hajnoczi 2026-06-03 16:51 ` Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2026-06-01 19:12 UTC (permalink / raw) To: Denis V. Lunev Cc: Denis V. Lunev, qemu-devel, Markus Armbruster, Paolo Bonzini, Laurent Vivier, Helge Deller, Warner Losh [-- Attachment #1: Type: text/plain, Size: 2126 bytes --] On Thu, May 28, 2026 at 11:24:48AM +0200, Denis V. Lunev wrote: > On 5/20/26 23:26, Denis V. Lunev wrote: > > A bug report against our downstream tree turned out to have its root > > cause in plain mainstream code: envlist_unsetenv() does a prefix-match > > lookup that drops the wrong entry when one stored name happens to be a > > prefix of another. The downstream symptom is specific to our setup and > > isn't interesting here -- the underlying lookup mistake is the part > > worth fixing, and it is reachable from a normal qemu-user invocation > > through the -U command-line option, so the fix belongs upstream. > > > > Patch 1 fixes the lookup: each entry now stores its name length at > > insertion time, and a tiny helper compares with explicit length > > equality plus memcmp. envlist_setenv()'s self-search was accidentally > > safe (it included the '=' byte in its strncmp window and that '=' > > served as a boundary) but is converted to the same helper so the name > > boundary becomes a structural property of the entry rather than a > > property of its byte layout. Without that, the two sites can easily > > drift apart again. > > > > Patch 2 backfils test coverage for util/envlist -- there was none -- > > in tests/unit/test-envlist. I verified that the regression case > > (envlist_unsetenv("FOO") vs. a stored "FOOBAR=...") fails against the > > pre-fix code and passes after the fix. > > > > Signed-off-by: Denis V. Lunev <den@openvz.org> > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > Cc: Markus Armbruster <armbru@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > > Denis V. Lunev (2): > > util/envlist: fix prefix-match in envlist_unsetenv() name lookup > > tests/unit: add test-envlist covering setenv/unsetenv name matching > > > > tests/unit/meson.build | 1 + > > tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++ > > util/envlist.c | 19 +++- > > 3 files changed, 212 insertions(+), 4 deletions(-) > > create mode 100644 tests/unit/test-envlist.c CCing Laurent, Helge, and Warner in case they are interested in this -user fix. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev 2026-06-01 19:12 ` Stefan Hajnoczi @ 2026-06-01 19:18 ` Stefan Hajnoczi 2026-06-03 16:51 ` Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2026-06-01 19:18 UTC (permalink / raw) To: Denis V. Lunev Cc: Denis V. Lunev, qemu-devel, Markus Armbruster, Paolo Bonzini [-- Attachment #1: Type: text/plain, Size: 2195 bytes --] On Thu, May 28, 2026 at 11:24:48AM +0200, Denis V. Lunev wrote: > On 5/20/26 23:26, Denis V. Lunev wrote: > > A bug report against our downstream tree turned out to have its root > > cause in plain mainstream code: envlist_unsetenv() does a prefix-match > > lookup that drops the wrong entry when one stored name happens to be a > > prefix of another. The downstream symptom is specific to our setup and > > isn't interesting here -- the underlying lookup mistake is the part > > worth fixing, and it is reachable from a normal qemu-user invocation > > through the -U command-line option, so the fix belongs upstream. > > > > Patch 1 fixes the lookup: each entry now stores its name length at > > insertion time, and a tiny helper compares with explicit length > > equality plus memcmp. envlist_setenv()'s self-search was accidentally > > safe (it included the '=' byte in its strncmp window and that '=' > > served as a boundary) but is converted to the same helper so the name > > boundary becomes a structural property of the entry rather than a > > property of its byte layout. Without that, the two sites can easily > > drift apart again. > > > > Patch 2 backfils test coverage for util/envlist -- there was none -- > > in tests/unit/test-envlist. I verified that the regression case > > (envlist_unsetenv("FOO") vs. a stored "FOOBAR=...") fails against the > > pre-fix code and passes after the fix. > > > > Signed-off-by: Denis V. Lunev <den@openvz.org> > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > Cc: Markus Armbruster <armbru@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > > Denis V. Lunev (2): > > util/envlist: fix prefix-match in envlist_unsetenv() name lookup > > tests/unit: add test-envlist covering setenv/unsetenv name matching > > > > tests/unit/meson.build | 1 + > > tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++ > > util/envlist.c | 19 +++- > > 3 files changed, 212 insertions(+), 4 deletions(-) > > create mode 100644 tests/unit/test-envlist.c > > I will way until Wednesday to merge this so that the -user maintainers have a chance to take a look: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev 2026-06-01 19:12 ` Stefan Hajnoczi 2026-06-01 19:18 ` Stefan Hajnoczi @ 2026-06-03 16:51 ` Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2026-06-03 16:51 UTC (permalink / raw) To: Denis V. Lunev Cc: Denis V. Lunev, qemu-devel, Markus Armbruster, Paolo Bonzini [-- Attachment #1: Type: text/plain, Size: 2187 bytes --] On Thu, May 28, 2026 at 11:24:48AM +0200, Denis V. Lunev wrote: > On 5/20/26 23:26, Denis V. Lunev wrote: > > A bug report against our downstream tree turned out to have its root > > cause in plain mainstream code: envlist_unsetenv() does a prefix-match > > lookup that drops the wrong entry when one stored name happens to be a > > prefix of another. The downstream symptom is specific to our setup and > > isn't interesting here -- the underlying lookup mistake is the part > > worth fixing, and it is reachable from a normal qemu-user invocation > > through the -U command-line option, so the fix belongs upstream. > > > > Patch 1 fixes the lookup: each entry now stores its name length at > > insertion time, and a tiny helper compares with explicit length > > equality plus memcmp. envlist_setenv()'s self-search was accidentally > > safe (it included the '=' byte in its strncmp window and that '=' > > served as a boundary) but is converted to the same helper so the name > > boundary becomes a structural property of the entry rather than a > > property of its byte layout. Without that, the two sites can easily > > drift apart again. > > > > Patch 2 backfils test coverage for util/envlist -- there was none -- > > in tests/unit/test-envlist. I verified that the regression case > > (envlist_unsetenv("FOO") vs. a stored "FOOBAR=...") fails against the > > pre-fix code and passes after the fix. > > > > Signed-off-by: Denis V. Lunev <den@openvz.org> > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > Cc: Markus Armbruster <armbru@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > > > Denis V. Lunev (2): > > util/envlist: fix prefix-match in envlist_unsetenv() name lookup > > tests/unit: add test-envlist covering setenv/unsetenv name matching > > > > tests/unit/meson.build | 1 + > > tests/unit/test-envlist.c | 196 ++++++++++++++++++++++++++++++++++++++ > > util/envlist.c | 19 +++- > > 3 files changed, 212 insertions(+), 4 deletions(-) > > create mode 100644 tests/unit/test-envlist.c > > > ping > Thanks, applied to staging: https://gitlab.com/qemu-project/qemu/commits/staging Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-03 16:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-20 21:26 [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 1/2] util/envlist: fix prefix-match in envlist_unsetenv() " Denis V. Lunev via qemu development 2026-05-20 21:26 ` [PATCH 2/2] tests/unit: add test-envlist covering setenv/unsetenv name matching Denis V. Lunev via qemu development 2026-05-28 9:24 ` [PATCH 0/2] util/envlist: fix prefix-match in name lookup Denis V. Lunev 2026-06-01 19:12 ` Stefan Hajnoczi 2026-06-01 19:18 ` Stefan Hajnoczi 2026-06-03 16:51 ` Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.