"Daniel P. Berrange" <berra...@redhat.com> 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 <arm...@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
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!