possible use-after-free in amdgpu_dm

[Andrey Grodzovsky]

On 10/17/2017 01:58 PM, Felix Kuehling wrote:
> 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?

I don't have an answer for why not just memset, this hook is a an older 
relic and not implemented
properly, dm_drm_plane_reset is a more recent example of how it should 
be implemented
(calling atomic_destroy_state hook inside) and i think we do kfree there 
just because DRM's helpers
(e.g. drm_atomic_helper_plane_destroy_state) do the same.

Thanks,
Andrey

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