linux-kernel - Re: [PATCH 1/2] media: venus: fix OOB read issue due to double read

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: <5651a3dd-79bb-4711-98bd-e1932f32ac23@quicinc.com>
Date: Wed, 29 Jan 2025 10:30:28 +0530
From: Vedang Nagar <quic_vnagar@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Stanimir Varbanov
	<stanimir.k.varbanov@...il.com>,
        Vikash Garodia <quic_vgarodia@...cinc.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
CC: <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] media: venus: fix OOB read issue due to double read

Hi Bryan,

On 1/17/2025 3:55 PM, Bryan O'Donoghue wrote:
> On 17/01/2025 08:39, Vedang Nagar wrote:
>> Below is the first read where dwords is being validated properly with the checks.
>> dwords = *rd_ptr >> 2;
>>
>> Whereas the same address is being read for the second time:
>> memcpy(pkt, rd_ptr, dwords << 2);
>>
>> For the second read the value is not validated which may get updated from the firmware
>> leading to incorrect memcpy into the packet and may lead to OOB read access while accessing
>> the packet.
> 
> So you are saying that pkt points to memory that the firmware and host can simultaneously access.
> 
> The question is - if the length value can change between one read and another read - how do you trust the _content_ of the packet ?
Original content of the packet is validated while reading the packet in hfi_process_msg_packet function.
Whereas the current change is just to validate the size of the packet to avoid the Out of bound read access.
> 
> Surely the right thing to do is to take a _copy_ of the entire frame and act on that frame exclusively on the host side ?
> 
> If I receive a frame, and read length X.
> 
> Then I need to re-read that frame because length may now by X+3.
> 
> This implies the _data_ in the frame has changed.
Yes, the _data_ in the frame has changed and will get rejected while parsing that data.
So I think it's okay to no read or copy the entire frame again.
> 
> What exactly is the valid lifetime of this data from HFI RX interrupt ?
There is no as such lifetime of the interrupt, but any rogue firmware can corrupt the data in the packet.

Regards,
Vedang Nagar
> 
> ---
> bod