possible use-after-free in amdgpu_dm

[Tom St Denis]

On 17/10/17 01:23 PM, Tom St Denis wrote:
> On 17/10/17 01:18 PM, Christian König wrote:
>> Am 17.10.2017 um 16:10 schrieb Tom St Denis:
>>> In this block of code:
>>>
>>> void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector)
>>> {
>>>     struct dm_connector_state *state =
>>>         to_dm_connector_state(connector->state);
>>>
>>>     kfree(state);
>>>
>>>     state = kzalloc(sizeof(*state), GFP_KERNEL);
>>>
>>>
>>> The value of state is never compared with NULL and moreso the value 
>>> of connector->state is never written to if NULL. Wouldn't this mean 
>>> the pointer points to freed memory?
>>
>> Why should we compare the value of state to NULL? What's done here is 
>> just to get the size of the type state points to.
>>
>> Not sure if that is really covered by the C standard, but in practice 
>> it works fine even when state is NULL.
> 
> Hi Christian,
> 
> 
> Oh sorry no the issue isn't with the sizeof (that's perfectly fine since 
> the standard says the pointer won't be dereferenced).
> 
> The issue is later on in the function there's this statement:
> 
>          connector->state = &state->base;
> 
> Where "base" is first item in the struct so it's effectively just 
> "connector->state = &state".
> 
> This means that the value of "connector->state" is stale since the 
> pointer was kfree'ed right (if the alloc fails) which could lead to a 
> use-after-free error (I don't know where this function lies in the rest 
> of the code paths but it seems wrong either way).


Sorry I think I might be explaining this poorly.

In the case the alloc succeeds the pointer is updated and everything is 
fine.

IF the alloc fails the pointer (connector->state) is not updated and the 
value points to freed memory.

Tom