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: <20240304173120.GP20455@kvack.org>
Date: Mon, 4 Mar 2024 12:31:20 -0500
From: Benjamin LaHaise <ben@...munityfibre.ca>
To: Bart Van Assche <bvanassche@....org>
Cc: Edward Adam Davis <eadavis@...com>,
	syzbot+b91eb2ed18f599dd3c31@...kaller.appspotmail.com,
	brauner@...nel.org, jack@...e.cz, linux-aio@...ck.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	syzkaller-bugs@...glegroups.com, viro@...iv.linux.org.uk
Subject: Re: [PATCH] fs/aio: fix uaf in sys_io_cancel

On Mon, Mar 04, 2024 at 09:15:47AM -0800, Bart Van Assche wrote:
> On 3/4/24 09:03, Benjamin LaHaise wrote:
> >This is just so wrong there aren't even words to describe it.  I
> >recommending reverting all of Bart's patches since they were not reviewed
> >by anyone with a sufficient level of familiarity with fs/aio.c to get it
> >right.
> 
> Where were you while my patches were posted for review on the fsdevel
> mailing list and before these were sent to Linus?

That doesn't negate the need for someone with experience in a given
subsystem to sign off on the patches.  There are at least 2 other people I
would trust to properly review these patches, and none of their signoffs
are present either.

> A revert request should include a detailed explanation of why the revert
> should happen. Just claiming that something is wrong is not sufficient
> to motivate a revert.

A revert is justified when a series of patches is buggy and had
insufficient review prior to merging.  Mainline is not the place to be
testing half baked changes.  Getting cancellation semantics and locking
right is *VERY HARD*, and the results of getting it wrong are very subtle
and user exploitable.  Using the "a kernel warning hit" approach for work
on cancellation is very much a sign that the patches were half baked.

What testing did you do on your patch series?  The commit messages show
nothing of interest regarding testing.  Why are you touching the kiocb
after ownership has already been passed on to another entity?  This is as
bad as touching memory that has been freed.  You clearly do not understand
how that code works.

		-ben
-- 
"Thought is the essence of where you are now."

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ