[Mesa-dev] [Mesa-stable] [PATCH] st/clover: Don't segfault on clCreateBuffer with null context

[Aaron Watry]

Yeah, that looks good to me.  Several of the function headers already
mention similar validation, but it's nice to see it in plain language.
 Hopefully it's not a mistake I'll make again.

--Aaron

On Fri, Apr 4, 2014 at 7:12 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Aaron Watry <awatry at gmail.com> writes:
>
>> I guess I misunderstood that obj() call (my IDE is misbehaving on the
>> machine I was doing that work on which limited my ability to easily
>> find function definitions outside of the current file).  C++ is pretty
>> unfamiliar to me and I guess that I figured that the obj() was some
>> alternative typecasting operation related to the typing for ctx not an
>> actual function call).  This'll teach me to stay away from C++ for a
>> while (again).
>>
>
> I see.  Maybe something like the attached patch will help avoid future
> misunderstandings?
>
>> The segfault that I was actually encountering ended up being in
>> ocl-icd (version in the Ubuntu saucy repo).  When I built that project
>> straight from git, the segfault went away (there's been quite a few
>> null deref fixes in there since the version that Ubuntu 13.10
>> packages).  This was after I had already gone through the memory.cpp
>> code and broken my understanding of what it was doing.
>>
>> FYI: I was attempting to run the create-buffer piglit CL API tests
>> (I'm working on a new extension of that for clCreateSubBuffer
>> testing).  It was disconcerting that the cl-api-create-buffer program
>> segfaulted reliably when encountering the test that attempts to
>> clCreateBuffer using a NULL context (line 264 in create-buffer.c).
>>
>> Sorry for the noise.
>>
>> --Aaron
>>
>> On Thu, Apr 3, 2014 at 6:54 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Aaron Watry <awatry at gmail.com> writes:
>>>
>>>> This should fix a segfault in the case that you're not using ocl-icd.
>>>>
>>>> If you're using ocl-icd, make sure your version is new enough or you'll get
>>>> the segfault before you even get to clover.
>>>>
>>>> The null de-ref seems to have been introduced in 10.1, but earlier versions
>>>> have have also been affected in other ways.
>>>>
>>>
>>> NAK.  The obj() function already checks if the pointer is NULL.  Not
>>> sure how you're getting a segfault.
>>>
>>>> CC: "10.1" <mesa-stable at lists.freedesktop.org>
>>>>
>>>> Signed-off-by: Aaron Watry <awatry at gmail.com>
>>>> ---
>>>>  src/gallium/state_trackers/clover/api/memory.cpp | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/src/gallium/state_trackers/clover/api/memory.cpp b/src/gallium/state_trackers/clover/api/memory.cpp
>>>> index 7ed2191..dad5ad3 100644
>>>> --- a/src/gallium/state_trackers/clover/api/memory.cpp
>>>> +++ b/src/gallium/state_trackers/clover/api/memory.cpp
>>>> @@ -32,6 +32,9 @@ clCreateBuffer(cl_context d_ctx, cl_mem_flags flags, size_t size,
>>>>                 void *host_ptr, cl_int *r_errcode) try {
>>>>     auto &ctx = obj(d_ctx);
>>>>
>>>> +   if (d_ctx == NULL)
>>>> +        throw error(CL_INVALID_CONTEXT);
>>>> +
>>>>     if (bool(host_ptr) != bool(flags & (CL_MEM_USE_HOST_PTR |
>>>>                                         CL_MEM_COPY_HOST_PTR)))
>>>>        throw error(CL_INVALID_HOST_PTR);
>>>> --
>>>> 1.8.3.2
>>>>
>>>> _______________________________________________
>>>> mesa-stable mailing list
>>>> mesa-stable at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
>