linux-kernel - Re: [RFC PATCH v9 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]

Date:   Thu, 23 Apr 2020 17:51:47 -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 v9 6/9] media: tegra: Add Tegra210 Video input driver


On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 24.04.2020 02:50, Sowjanya Komatineni пишет:
>> On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> 24.04.2020 02:20, Sowjanya Komatineni пишет:
>>>> On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
>>>>> On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> 22.04.2020 09:18, Sowjanya Komatineni пишет:
>>>>>>> +static int chan_capture_kthread_start(void *data)
>>>>>>> +{
>>>>>>> +     struct tegra_vi_channel *chan = data;
>>>>>>> +     struct tegra_channel_buffer *buf;
>>>>>>> +     int err = 0;
>>>>>>> +
>>>>>>> +     set_freezable();
>>>>>>> +
>>>>>>> +     while (1) {
>>>>>>> +             try_to_freeze();
>>>>>>> +
>>>>>>> +             wait_event_interruptible(chan->start_wait,
>>>>>>> + !list_empty(&chan->capture) ||
>>>>>>> +                                      kthread_should_stop());
>>>>>>> +
>>>>>>> +             if (kthread_should_stop())
>>>>>>> +                     break;
>>>>>>> +
>>>>>>> +             /*
>>>>>>> +              * Source is not streaming if error is non-zero.
>>>>>>> +              * So, do not dequeue buffers on capture error.
>>>>>>> +              */
>>>>>>> +             if (err)
>>>>>>> +                     continue;
>>>>>> This will result in an endless loop, I suppose it wasn't the
>>>>>> intention.
>>>>> no it will not. on error we report vb2_queue_error which will do
>>>>> streaming stop request.
>>>>>
>>>>> So thread will be stopped on streaming stop request thru kthread stop
>>>>> signal
>>>> To be clear on error it reports vb2 queue error and waits for stop
>>>> streaming to happen
>>> If thread should exit on error, then it should do it on the actual
>>> error. Otherwise it looks very error-prone.
>> When v4l2 drivers indicate fatal error through vb2_queue_error, queue
>> error flag  is set and wakes up all processes waiting on queue along
>> with polling reporting  EPOLLERR and also reporting error for queuing
>> and dequeuing buffers. Stream stop will surely happen which stops the
>> thread.
> This doesn't explain what is the point of continuing to loop instead of
> exiting immediately on error.

We are using 2 threads and when capture start error happens, we can stop 
capture_start thread immediately but capture_finish thread will still 
run for any outstanding buffers.

So, as it makes no diff stopping both threads during stream stop which 
will definitely happen on error and when we don't dequeue buffers