Re: [Qemu-trivial] [Qemu-devel] [PATCH] ui/vnc: Convert sasl.mechlist to g_malloc() & friends

["Daniel P. Berrange" <berrange@redhat.com>]

On Tue, Nov 08, 2011 at 10:55:52AM +0100, Markus Armbruster wrote:
> Fixes protocol_client_auth_sasl_mechname() not to crash when malloc()
> fails.  Spotted by Coverity.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/vnc-auth-sasl.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index 23b1bf5..a88973b 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -35,7 +35,7 @@ void vnc_sasl_client_cleanup(VncState *vs)
>          vs->sasl.encodedLength = vs->sasl.encodedOffset = 0;
>          vs->sasl.encoded = NULL;
>          g_free(vs->sasl.username);
> -        free(vs->sasl.mechlist);
> +        g_free(vs->sasl.mechlist);
>          vs->sasl.username = vs->sasl.mechlist = NULL;
>          sasl_dispose(&vs->sasl.conn);
>          vs->sasl.conn = NULL;
> @@ -430,7 +430,7 @@ static int protocol_client_auth_sasl_start_len(VncState *vs, uint8_t *data, size
>  
>  static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_t len)
>  {
> -    char *mechname = malloc(len + 1);
> +    char *mechname = g_malloc(len + 1);
>      if (!mechname) {
>          VNC_DEBUG("Out of memory reading mechname\n");
>          vnc_client_error(vs);

You can delete the   if (!mechname) block now you have g_malloc

The reason for the crash on OOM is here, but the diff context doesn't show it:

Notice the missing 'return -1'  statement following vnc_client_error(vs);

    char *mechname = malloc(len + 1);
    if (!mechname) {
        VNC_DEBUG("Out of memory reading mechname\n");
        vnc_client_error(vs);
    }
    strncpy(mechname, (char*)data, len);
    mechname[len] = '\0';



> @@ -460,7 +460,7 @@ static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_
>          }
>      }
>  
> -    free(vs->sasl.mechlist);
> +    g_free(vs->sasl.mechlist);
>      vs->sasl.mechlist = mechname;
>  
>      VNC_DEBUG("Validated mechname '%s'\n", mechname);
> @@ -469,7 +469,7 @@ static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_
>  
>   fail:
>      vnc_client_error(vs);
> -    free(mechname);
> +    g_free(mechname);
>      return -1;
>  }
>  
> @@ -608,7 +608,7 @@ void start_auth_sasl(VncState *vs)
>      }
>      VNC_DEBUG("Available mechanisms for client: '%s'\n", mechlist);
>  
> -    if (!(vs->sasl.mechlist = strdup(mechlist))) {
> +    if (!(vs->sasl.mechlist = g_strdup(mechlist))) {
>          VNC_DEBUG("Out of memory");
>          sasl_dispose(&vs->sasl.conn);
>          vs->sasl.conn = NULL;

Again, you can delete the conditional here with g_strdup

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