[PATCH 0/2] util/envlist: fix prefix-match in name lookup

Denis V. Lunev via qemu development posted 2 patches 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260520212628.479772-1-den@openvz.org
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
[PATCH 0/2] util/envlist: fix prefix-match in name lookup
Posted by Denis V. Lunev via qemu development 1 month, 2 weeks ago
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
Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup
Posted by Denis V. Lunev 1 month, 1 week ago
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
Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup
Posted by Stefan Hajnoczi 1 month, 1 week ago
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
Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup
Posted by Stefan Hajnoczi 1 month, 1 week ago
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>
Re: [PATCH 0/2] util/envlist: fix prefix-match in name lookup
Posted by Stefan Hajnoczi 1 month, 1 week ago
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