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

Re: [libvirt] [PATCH] virnettlscontext: Resolve Coverity warnings (UNINIT)



On Fri, Aug 09, 2013 at 07:19:18AM -0400, John Ferlan wrote:
> Coverity complained about the usage of the uninitialized cacerts in the
> event(s) that "access(certFile, R_OK)" and/or "access(cacertFile, R_OK)"
> fail the for loop used to fill in the certs will have indeterminate data
> as well as the possibility that both failures would result in the
> gnutls_x509_crt_deinit() call having a similar fate.
> 
> Initializing cacerts only would resolve the issue; however, it still
> would leave the indeterminate action, so rather add a parameter to
> the virNetTLSContextLoadCACertListFromFile() to pass the max size rather
> then overloading the returned count parameter. If the the call is never
> made, then we won't go through the for loops referencing the empty
> cacerts
> ---
>  src/rpc/virnettlscontext.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index 2beee8f..7cee27c 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -545,12 +545,12 @@ cleanup:
>  
>  static int virNetTLSContextLoadCACertListFromFile(const char *certFile,
>                                                    gnutls_x509_crt_t *certs,
> +                                                  unsigned int certMax,
>                                                    size_t *ncerts)
>  {
>      gnutls_datum_t data;
>      char *buf = NULL;
>      int ret = -1;
> -    unsigned int certMax = *ncerts;
>  
>      *ncerts = 0;
>      VIR_DEBUG("certFile %s", certFile);
> @@ -584,15 +584,17 @@ static int virNetTLSContextSanityCheckCredentials(bool isServer,
>  {
>      gnutls_x509_crt_t cert = NULL;
>      gnutls_x509_crt_t cacerts[MAX_CERTS];
> -    size_t ncacerts = MAX_CERTS;
> +    size_t ncacerts = 0;
>      size_t i;
>      int ret = -1;
>  
> +    memset(cacerts, 0, sizeof(cacerts));
>      if ((access(certFile, R_OK) == 0) &&
>          !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer)))
>          goto cleanup;
>      if ((access(cacertFile, R_OK) == 0) &&
> -        virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, &ncacerts) < 0)
> +        virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts,
> +                                               MAX_CERTS, &ncacerts) < 0)
>          goto cleanup;
>  
>      if (cert &&

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]