linux-kernel - Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver

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]

Message-ID: <a69a8b34-beea-3ad0-e08e-f7df8b9e7047@nvidia.com>
Date:   Wed, 15 Apr 2020 09:54:35 -0700
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>, <thierry.reding@...il.com>,
        <jonathanh@...dia.com>, <frankc@...dia.com>, <hverkuil@...all.nl>,
        <sakari.ailus@....fi>, <helen.koike@...labora.com>
CC:     <sboyd@...nel.org>, <linux-media@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v7 6/9] media: tegra: Add Tegra210 Video input driver

On 4/15/20 7:22 AM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 15.04.2020 05:57, Sowjanya Komatineni пишет:
>> +static int tegra_csi_remove(struct platform_device *pdev)
>> +{
>> +     struct tegra_csi *csi = platform_get_drvdata(pdev);
>> +     int err;
>> +
>> +     err = host1x_client_unregister(&csi->client);
>> +     if (err < 0) {
>> +             dev_err(csi->dev,
>> +                     "failed to unregister host1x client: %d\n", err);
>> +             return err;
>> +     }
>> +
>> +     pm_runtime_disable(csi->dev);
>> +     kfree(csi);
> IIRC, the driver removal is invoked on the unbinding. Hence, I'm not
> sure how moving away from the resource-managed API helps here. Could you
> please explain in a more details?
>
> Have you tried to test this driver under KASAN? I suspect that you just
> masked the problem, instead of fixing it.
Using devm_kzalloc for vi/csi structures based on prior feedback request 
to switch to use kzalloc all over this driver.

Hi Hans,

video devices lifetime is till video device nodes are released. So, v4l2 
device release callback does the release of tegra channel allocation 
which hold video device.

Below are the 3 possible cases of unbind/unload,

1. during tegra-video module unload, if v4l2 device refcnt is not 0 
which is the case when any of video device node handle is kept opened 
then unloading module will not happen and module refcnt is also non-zero 
and unloading tegra-video module reports module in use.

2. during tegra-video driver unbind, tegra-video driver removal will do 
vi/csi clients exit ops which unregisters video device allocated memory 
during release callback of v4l2 device. vi/csi structure allocation 
remains same as vi/csi driver removal will not happen in this case.

3. during direct host1x client drivers vi/csi unbind, both 
host1x_clients vi/csi gets unregistered, deletes host1x logical device 
which executes tegra-video driver removal() -> vi/csi exit() before 
vi/csi memory gets freed in vi/csi driver remove().

So, any active streaming will stop and video devices are unregistered 
during direct client driver unbind prior to freeing vi/csi memory.

Also vi/csi driver remove does explicit free vi/csi as its allocated 
with kzalloc. So not sure how using kzalloc is different to devm_kzalloc 
for vi/csi structure in terms of when vi/csi memory gets freed?

Except for channel allocation which holds video device and as video 
device life time is beyond tegra-video module unbind->vi exit(), looks 
like we can use devm_kzalloc for vi/csi.

Can you please comment if you still think we need to use kzalloc rather 
than devm_kzalloc for vi/csi structure allocation?

Thanks

Sowjanya