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

Re: [libvirt] [PATCH 3/4] rpc: avoid freeing uninitialized variable



On Thu, Jun 30, 2011 at 08:14:55AM -0600, Eric Blake wrote:
> Detected by Coverity.  Both are instances of bad things happening
> if pipe2 fails; the virNetClientNew failure could free garbage,
> and virNetSocketNewConnectCommand could close random fds.
> 
> Note: POSIX doesn't guarantee the contents of fd[0] and fd[1]
> after pipe failure: http://austingroupbugs.net/view.php?id=467
> We may need to introduce a virPipe2 wrapper that guarantees
> that on pipe failure, the fds are explicitly set to -1, rather
> than our current state of assuming the fds are unchanged from
> their value prior to the failed pipe call.
> 
> * src/rpc/virnetclient.c (virNetClientNew): Initialize variable.
> * src/rpc/virnetsocket.c (virNetSocketNewConnectCommand):
> Likewise.
> ---
>  src/rpc/virnetclient.c |    2 +-
>  src/rpc/virnetsocket.c |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 39bdf14..b551b99 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -113,7 +113,7 @@ static void virNetClientIncomingEvent(virNetSocketPtr sock,
>  static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
>                                         const char *hostname)
>  {
> -    virNetClientPtr client;
> +    virNetClientPtr client = NULL;
>      int wakeupFD[2] = { -1, -1 };
> 
>      if (pipe2(wakeupFD, O_CLOEXEC) < 0) {
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index 96d2dfd..d16f8e5 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -510,8 +510,8 @@ int virNetSocketNewConnectCommand(virCommandPtr cmd,
>                                    virNetSocketPtr *retsock)
>  {
>      pid_t pid = 0;
> -    int sv[2];
> -    int errfd[2];
> +    int sv[2] = { -1, -1 };
> +    int errfd[2] = { -1, -1 };
> 
>      *retsock = NULL;
> 

ACK

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]