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

[Albert Freeman]

Reviewed-by: Albert Freeman <albertwdfreeman at gmail.com>

On 27 September 2015 at 09:41, Albert Freeman <albertwdfreeman at gmail.com> wrote:
> On 27 September 2015 at 09:35, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> 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
> Sorry, I was referencing slightly outdated mesa git code (without the goto).