linux-kernel - Re: [PATCH 2/2] media: venus: fix OOB access issue while reading sequence changed events

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: <7a782ea9-f227-4f46-a757-b4b69f5c287f@quicinc.com>
Date: Fri, 17 Jan 2025 14:09:37 +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 2/2] media: venus: fix OOB access issue while reading
 sequence changed events

Hi Bryan,

On 1/6/2025 5:36 AM, Bryan O'Donoghue wrote:
> On 04/01/2025 05:41, Vedang Nagar wrote:
>> num_properties_changed is being read from the message queue but is
>> not validated. Value can be corrupted from the firmware leading to
>> OOB read access issues. Add fix to read the size of the packets as
>> well and crosscheck before reading from the packet.
>>
>> Signed-off-by: Vedang Nagar <quic_vnagar@...cinc.com>
> Please see Vikash's series on this.
> 
> https://lore.kernel.org/linux-arm-msm/20241128-venus_oob_2-v2-2-483ae0a464b8@quicinc.com/
> 
> it seems to have exactly the same patch title ?
> 
> Is this patch supposed to be a follow-up to that patch ?
> 
> https://lore.kernel.org/linux-arm-msm/20241128-venus_oob_2-v2-0-483ae0a464b8@quicinc.com/
> 
> Expecting to see a V3 of the above. If the intention is to supersede that patch or some of those patches you should make clear here.
No, this is a different series having OOB fixes similar to ones posted by Vikash.
> 
> On the switch statement I'd have two comments.
> 
> #1 is everything really a " -= sizeof(u32)" ?
Yes, it's everytime " -= sizeof(u32) " since the first the first word read is ptype of size u32
> #2 if so then this ought to be factored out into a function
>    => functional decomposition
Sure, will fix this with decomposition into functions.

Regards,
Vedang Nagar
> 
> ---
> bod