Re: [PATCH 1/2] util: fix msan findings in keyval

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, May 09, 2025 at 07:48:57AM +0200, Markus Armbruster wrote:
> Nabih Estefan <nabihestefan@google.com> writes:
> 
> > From: Peter Foley <pefoley@google.com>
> >
> > e.g.
> > I	2025-02-28 09:51:05.240071-0800		624	stream.go:47	qemu: Uninitialized value was created by an allocation of 'key_in_cur.i' in the stack frame
> > I	2025-02-28 09:51:05.240187-0800		624	stream.go:47	qemu: #0 0xaaaac49f489c in keyval_parse_one third_party/qemu/util/keyval.c:190:5
> >
> > Signed-off-by: Peter Foley <pefoley@google.com>
> > Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> >  util/keyval.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/util/keyval.c b/util/keyval.c
> > index a70629a481..f33c64079d 100644
> > --- a/util/keyval.c
> > +++ b/util/keyval.c
> > @@ -187,7 +187,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
> >  {
> >      const char *key, *key_end, *val_end, *s, *end;
> >      size_t len;
> > -    char key_in_cur[128];
> > +    char key_in_cur[128] = {};
> >      QDict *cur;
> >      int ret;
> >      QObject *next;
> 
> Prior review of Peter's patch concluded this must be false positive:
> https://lore.kernel.org/qemu-devel/14168384-ecdb-4c05-8267-ac5ef1c46fe9@redhat.com/

While I agree with Paolo's reasoning, I think it is still worth adding an
explicit initializer, because it makes it easier for both humans and machines
to reason about correctless.

To reinforce that we don't have an actual bug though, also note that qemu
unconditionally builds with -ftrivial-auto-var-init=zero. So if we happen
to forget any, it won't cause a bug in the common case of a zero-initializer.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|