qemu-devel
[Top][All Lists]
Advanced

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

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH] ui/vnc: Convert sasl.mechlist to g_malloc() & friends
Date: Tue, 08 Nov 2011 11:48:50 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux)

"Daniel P. Berrange" <address@hidden> writes:

> 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 <address@hidden>
>> ---
>>  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

Should've seen that myself.  Guess I stared at Coverity reports for too
long.  I'll respin.

> 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';

Correct.

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

Yes.

Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]