On 10/17/2017 03:43 PM, Harry Wentland wrote:
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?

Tom already sent a patch for that and i rb it.

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

I suggest first to align this hook with the dm_plane and dm_crtc hooks.

Thanks,
Andrey


Harry

Regards,
Christian.

Regards,
    Felix

Tom
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to