Re: [PATCH] drm/panfrost: Prevent race when handling page fault

[Steven Price <steven.price@arm.com>]

On 07/09/2019 20:36, Daniel Vetter wrote:
> On Fri, Sep 6, 2019 at 2:42 PM Steven Price <steven.price@arm.com> wrote:
>>
>> On 06/09/2019 12:10, Rob Herring wrote:
>>> On Thu, Sep 5, 2019 at 1:11 PM Steven Price <steven.price@arm.com> wrote:
>>>>
>>>> When handling a GPU page fault addr_to_drm_mm_node() is used to
>>>> translate the GPU address to a buffer object. However it is possible for
>>>> the buffer object to be freed after the function has returned resulting
>>>> in a use-after-free of the BO.
>>>>
>>>> Change addr_to_drm_mm_node to return the panfrost_gem_object with an
>>>> extra reference on it, preventing the BO from being freed until after
>>>> the page fault has been handled.
>>>>
>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>> ---
>>>>
>>>> I've managed to trigger this, generating the following stack trace.
>>>
>>> Humm, the assumption was that a fault could only happen during a job
>>> and so a reference would already be held. Otherwise, couldn't the GPU
>>> also be accessing the BO after it is freed?
>>
>> Ah, I guess I missed that in the commit message. This is assuming that
>> user space doesn't include the BO in the job even though the GPU then
>> does try to access it. AIUI mesa wouldn't do this, but this is still
>> easily possible if user space wants to crash the kernel.
> 
> Do we have some nice regression tests for uapi exploits and corner
> cases like this? Maybe even in igt?
> -Daniel

Not currently, I've been playing with the idea of getting the
closed-source DDK blob running on Panfrost and this is what generates
the "not-quite-mesa" usage.

It would definitely be good extend the test cases in IGT, I have a
synthetic test which can trigger this - I just need to get approval to
post it.

Steve