[libvirt] [PATCH 4/9] virutil: Resolve Coverity RESOURCE_LEAK

[Eric Blake]

[adding gnulib, in case anyone else runs into the same false positive]

On 09/11/2014 06:06 PM, John Ferlan wrote:
> This ends up being a very bizarre false positive. With an assist from
> eblake, the claim is that mgetgroups() could return a -1 value, but yet
> still have a groups buffer allocated, yet the example shown doesn't
> seem to prove that.
> 
> Rather than fret about it, by adding a well placed sa_assert() on the
> returned *list value we can "assure" ourselves that the mgetgroups()
> failure path won't signal this condition.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/util/virutil.c | 1 +
>  1 file changed, 1 insertion(+)

ACK.

More details that we worked out on IRC: the mgetgroups code gets a list
that may contain duplicates, so it makes a final pass to reduce two
types of duplicates - any later member with the first, or any two
adjacent members.

  if (1 < ng)
    {
      gid_t first = *g;
      gid_t *next;
      gid_t *groups_end = g + ng;

      for (next = g + 1; next < groups_end; next++)
        {
          if (*next == first || *next == *g)
            ng--;
          else
            *++g = *next;
        }
    }

Coverity was assuming that ng-- could be executed enough times to make
it go negative, but looking closer at the loop bounds, the loop cannot
run more than the original ng - 1 times, so ng >= 1.  I'm not sure if
upstream gnulib can or should do anything to silence Coverity from
triggering the false positive.  But the added assert in the caller
definitely lets Coverity fill in the gaps for what we know to be true.

> 
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 8d2f62a..5197969 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1063,6 +1063,7 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
>  
>      ret = mgetgroups(user, primary, list);
>      if (ret < 0) {
> +        sa_assert(!*list);
>          virReportSystemError(errno,
>                               _("cannot get group list for '%s'"), user);
>          goto cleanup;
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140911/e472055b/attachment-0001.sig>