possible use-after-free in amdgpu_dm

[Tom St Denis]

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

Tom