Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in _destroy()

From: Shuah Khan
Date: Thu Jun 16 2016 - 14:55:35 EST


On 06/16/2016 12:43 PM, Max Kellermann wrote:
> On 2016/06/16 18:24, Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> wrote:
>> On 06/15/2016 02:15 PM, Max Kellermann wrote:
>>> media_gobj_destroy() may be called twice on one instance - once by
>>> media_device_unregister() and again by dvb_media_device_free(). The
>>> function media_remove_intf_links() establishes and documents the
>>> convention that mdev==NULL means that the object is not registered,
>>> but nobody ever NULLs this variable. So this patch really implements
>>> this behavior, and adds another mdev==NULL check to
>>> media_gobj_destroy() to protect against double removal.
>>
>> Are you seeing null pointer dereference on gobj->mdev? In any case,
>> we have to look at if there is a missing mutex hold that creates a
>> race between media_device_unregister() and dvb_media_device_free()
>>
>> I don't this patch will solve the race condition.
>
> I think we misunderstand. This is not about a race condition. And
> the problem cannot be a NULL pointer dereference.
>
> That's because nobody NULLs the pointer!

I see 7 calls to media_gobj_destroy(). In 6 cases, calling routines
fee the pointer that contains the graph_obj.

__media_device_unregister_entity() sets mdev ot null.

entity->graph_obj.mdev = NULL;

That is why I am confused when you say it never set to null.

thanks,
-- Shuah