possible use-after-free in amdgpu_dm

[Harry Wentland]

On 2017-10-17 02:15 PM, Christian König wrote:
> Am 17.10.2017 um 19:58 schrieb Felix Kuehling:
>> On 2017-10-17 01:25 PM, Tom St Denis wrote:
>>> 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?
>>> 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.
>> I'm wondering why the function frees, and then reallocates the memory.
>> Does its size change? If not, why not just memset it to 0?
> 
> Yeah, I was just thinking the same thing.
> 
> And no from the code snippet Tom posted it just frees and reallocated the same structure.
> 
> I think that should rather look like this:
> 
> if (!state)
>      state = kzalloc(...);
> else
>     memset(...);
> 

I don't expect connector->state to ever be NULL here which seems to be confirmed by
code that would have blown up if it ever was. How about we do a BUG_ON on this condition
as has been suggested?

Otherwise I like the memset idea. No need for free/alloc here.

Harry

> Regards,
> Christian.
> 
>>
>> Regards,
>>    Felix
>>
>>> Tom
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx