Re: [PATCH 2/2] _gsasl_gssapi_server_step: avoid empty challenge

[Andreas Oberritter]

On 26.10.2011 22:04, Simon Josefsson wrote:
> I think your patch is fine, except that I believe there is a memory leak
> of a newly allocated zero-length buffer (sic!).  I suspect it is
> possible to allocate a zero-page buffer on some systems, and that it has
> to be released.  How about this modified patch?  It is not as simple as
> yours, but I couldn't think of a better way to structure the code and
> also deal with the memory leak.

I'm fine with this version.

Thanks,
Andreas

> diff --git a/lib/gssapi/server.c b/lib/gssapi/server.c
> index f76430e..9bea963 100644
> --- a/lib/gssapi/server.c
> +++ b/lib/gssapi/server.c
> @@ -165,18 +165,25 @@ _gsasl_gssapi_server_step (Gsasl_session * sctx,
>        if (maj_stat == GSS_S_COMPLETE)
>       state->step++;
>  
> -      *output = malloc (bufdesc2.length);
> -      if (!*output)
> -     return GSASL_MALLOC_ERROR;
> -      memcpy (*output, bufdesc2.value, bufdesc2.length);
> -      *output_len = bufdesc2.length;
> +      if (maj_stat == GSS_S_CONTINUE_NEEDED || bufdesc2.length > 0)
> +     {
> +       *output = malloc (bufdesc2.length);
> +       if (!*output)
> +         return GSASL_MALLOC_ERROR;
> +       memcpy (*output, bufdesc2.value, bufdesc2.length);
> +       *output_len = bufdesc2.length;
> +     }
>  
>        maj_stat = gss_release_buffer (&min_stat, &bufdesc2);
>        if (GSS_ERROR (maj_stat))
>       return GSASL_GSSAPI_RELEASE_BUFFER_ERROR;
>  
> -      res = GSASL_NEEDS_MORE;
> -      break;
> +      if (maj_stat == GSS_S_CONTINUE_NEEDED || *output_len > 0)
> +     {
> +       res = GSASL_NEEDS_MORE;
> +       break;
> +     }
> +      /* fall through */
>  
>      case 2:
>        memset (tmp, 0xFF, 4);