linux-kernel - Re: [PATCHv6 0/3] media: venus: close() fixes

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: <20241105133843.GA13546@google.com>
Date: Tue, 5 Nov 2024 22:38:43 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Stanimir Varbanov <svarbanov@...e.de>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>,
	Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
	Vikash Garodia <quic_vgarodia@...cinc.com>,
	Dikshita Agarwal <quic_dikshita@...cinc.com>,
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv6 0/3] media: venus: close() fixes

Hi Stanimir,

On (24/11/05 14:04), Stanimir Varbanov wrote:
> On 10/25/24 19:56, Sergey Senozhatsky wrote:
> > A couple of fixes for venus driver close() handling
> > (both enc and dec).
> > 
> > v5->v6:
> > -- added kfree() backtrace to 0002
> > 
> > Sergey Senozhatsky (3):
> >   media: venus: fix enc/dec destruction order
> >   media: venus: sync with threaded IRQ during inst destruction
> >   media: venus: factor out inst destruction routine
> 
> Could you please combine 1/3 and 2/3 commit bodies into 3/3 body and
> resend the new 3/3 only. I do not see a reason to apply 1/3 and 2/3.

So the reason being is that 1/3 fixes a race condition (stale data
in ->fh) and a lockdep splat (wrong destruction order).  2/3 fixes a
completely different race (IRQ vs close) condition and UAF.  And 3/3
is just a refactoring that doesn't fix anything.  Are you sure you
want to squash all 3 of them?  Because they look slightly independent
to me.

> Also, on what platform this was tested?

I ran CTS on one of the strongbad devices.