linux-kernel - Re: [PATCH v2 01/23] media: iris: Skip destroying internal buffer if not dequeued

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: <2c431232-e0d6-1e6c-cd22-a912b5f08f7a@quicinc.com>
Date: Tue, 29 Apr 2025 16:28:00 +0530
From: Dikshita Agarwal <quic_dikshita@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Vikash Garodia
	<quic_vgarodia@...cinc.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        "Mauro Carvalho Chehab" <mchehab@...nel.org>,
        Stefan Schmidt
	<stefan.schmidt@...aro.org>,
        Hans Verkuil <hverkuil@...all.nl>,
        "Bjorn
 Andersson" <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        "Rob Herring" <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        "Conor Dooley" <conor+dt@...nel.org>
CC: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
        Neil Armstrong
	<neil.armstrong@...aro.org>,
        Nicolas Dufresne
	<nicolas.dufresne@...labora.com>,
        <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@...aro.org>,
        <20250424-qcs8300_iris-v5-0-f118f505c300@...cinc.com>,
        <stable@...r.kernel.org>
Subject: Re: [PATCH v2 01/23] media: iris: Skip destroying internal buffer if
 not dequeued



On 4/29/2025 3:13 PM, Bryan O'Donoghue wrote:
> On 28/04/2025 10:28, Dikshita Agarwal wrote:
>> +            /*
>> +             * during stream on, skip destroying internal(DPB) buffer
>> +             * if firmware did not return it.
>> +             * during close, destroy all buffers irrespectively.
>> +             */
>> +            if (!force && buf->attr & BUF_ATTR_QUEUED)
>> +                continue;
>> +
> 
> What's the effect of the firmware not having dequeued the buffer though ?
> 
> My main concern here is APSS and firmware have a different view of DMA memory.
> 
> We release on the APSS side but firmware has not.
> 
> Surely failure to release buffers by the time we get to Linux::close() is a
> failure of the software contract sufficient to require resetting the
> firmware ?
> 
> i.e. we release memory on the APSS side but firmware writes into it anyway ...
> 
As I mentioned earlier as well, during STOP issued before close, firmware
is expected to release all these internal buffers and the check I added now
in this patch will also ensures that.
Firmware also has a check to make sure all buffers queued are released as
part of STOP and then only firmware sends the STOP_DONE response to driver.
STOP is a synchronous call and driver doesn't proceed without receiving the
response for the same from firmware.
So we will never run into this issue where driver release buffers which are
still being accessed by firmware.
Please note, these are internal buffers which are allocated and managed by
driver only.

Thanks,
Dikshita
> ?
> 
> ---
> bod