linux-kernel - Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Jun 2016 12:55:28 -0600
From:	Shuah Khan <shuahkh@....samsung.com>
To:	linux-media@...r.kernel.org, mchehab@....samsung.com,
	linux-kernel@...r.kernel.org, Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in
 _destroy()

On 06/16/2016 12:43 PM, Max Kellermann wrote:
> On 2016/06/16 18:24, Shuah Khan <shuahkh@....samsung.com> 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