linux-kernel - Re: [PATCH 2/2] media: omap3isp change to devm for resources

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] [day] [month] [year] [list]

Date:   Fri, 16 Dec 2016 13:34:04 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Shuah Khan <shuahkh@....samsung.com>, sakari.ailus@...ux.intel.com,
        mchehab@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] media: omap3isp change to devm for resources

On 16/12/16 13:19, Laurent Pinchart wrote:
> Hi Hans,
>
> On Friday 16 Dec 2016 12:39:49 Hans Verkuil wrote:
>> On 15/12/16 20:40, Shuah Khan wrote:
>>> Using devm resources that have external dependencies such as a dev
>>> for a file handler could result in devm resources getting released
>>> durin unbind while an application has the file open holding pointer
>>> to the devm resource. This results in use-after-free errors when the
>>> application exits.
>>
>> That's solving the wrong problem.
>>
>> The real problem is that when registering a video_device it should do
>> this:
>>
>> devnode->cdev.kobj.parent = &devnode->dev.kobj;
>>
>> (taken from cec-core.c)
>>
>> This will prevent isp->dev from being released as long as there is a
>> filehandle still open.
>
> But it won't be enough, devm_* resources are released at unbind time, not at
> device release time. Right after the unbind (.remove() for platform devices)
> handler returns, devm_kzalloc allocated memory goes away.

You're completely right, I keep forgetting about that.

Sorry for the noise.

	Hans

>
>> After that change I believe that this will work correctly, but this
>> has to be tested first!
>