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

[Albert Freeman]

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).