[Mesa-dev] [PATCH] gallium/util: avoid unreferencing random memory on buffer alloc failure

[Ilia Mirkin]

On Sat, Sep 26, 2015 at 7:29 PM, Albert Freeman
<albertwdfreeman at gmail.com> wrote:
> On 27 September 2015 at 03:46, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> Found by Coverity
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/gallium/auxiliary/util/u_blitter.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c
>> index 2fbf69c..b7b1ece 100644
>> --- a/src/gallium/auxiliary/util/u_blitter.c
>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>> @@ -2065,7 +2065,7 @@ void util_blitter_clear_buffer(struct blitter_context *blitter,
>>     struct blitter_context_priv *ctx = (struct blitter_context_priv*)blitter;
>>     struct pipe_context *pipe = ctx->base.pipe;
>>     struct pipe_vertex_buffer vb = {0};
>> -   struct pipe_stream_output_target *so_target;
>> +   struct pipe_stream_output_target *so_target = NULL;
>>     unsigned offsets[PIPE_MAX_SO_BUFFERS] = {0};
>>
>>     assert(num_channels >= 1);
>> --
>> 2.4.9
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> "so_target = pipe->create_stream_output_target(pipe, dst, offset,
> size);" unconditionally assigns to so_target before first use (unless
> I am reading something incorrectly).
>

   u_upload_data(ctx->upload, 0, num_channels*4, clear_value,
                 &vb.buffer_offset, &vb.buffer);
   if (!vb.buffer)
      goto out;
...
   so_target = pipe->create_stream_output_target(pipe, dst, offset, size);
...
out:
...
   pipe_so_target_reference(&so_target, NULL);

If the goto out is taken, then &so_target gets unreferenced. This is
done by (roughly speaking) freeing the pointer that gets passed in,
and then assigning it to NULL.

> Perhaps all function pointers that don't get implemented by a driver
> should be zeroed? I am trying to think where this could be done (in
> the gallium drivers themselves? (although that doesn’t seem the ideal
> solution)). Perhaps in a function that is part of gallium itself
> (outside drivers/[state trackers]/etc) that gets called upon driver
> shared library load.

Most things are calloc'd and are thus initialized to null. Not sure
what that has to do with this issue though.

  -ilia