On Tue, Mar 15, 2011 at 08:03:09AM -0600, Eric Blake wrote:
> On 03/15/2011 06:32 AM, Daniel P. Berrange wrote:
> > The virCommandNewArgs() method would free the virCommandPtr
> > if it failed to add the args. This meant errors reported in
> > virCommandAddArgSet() were lost. Simply removing the check
> > for errors from the constructor means they can be reported
> > correctly later
> >
> > The virCommandAddEnvPassCommon() method failed to check for
> > errors before reallocating the cmd->env array, causing a
> > potential SEGV if cmd was NULL
> >
> > The virCommandAddArgSet() method needs to validate that at
> > least 1 element in 'val's parameter is non-NULL, otherwise
> > code like
> >
> > cmd = virCommandNew(binary)
> > virCommandAddAtg(cmd, "foo")
> >
> > Would end up trying todo execve("foo"), if binary was
> > NULL.
>
> Well, technically virCommandNew is ATTRIBUTE_NONNULL(1), so we would
> have caught this via clang (gcc's not quite as smart as clang at
> enforcing that parameter). But it doesn't hurt to be safe.
Yep, ATTRIBUTE_NONNULL is not really good enough as the only
line of defense - I did just hit this in one of my other
patch series.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
