[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix misc bugs in virCommandPtr



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]