linux-kernel - Re: [PATCH v5 15/40] netfs: Add support for DIO buffering

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]

Date: Thu, 28 Dec 2023 09:58:31 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Christian Brauner <brauner@...nel.org>
Cc: David Howells <dhowells@...hat.com>, Jeff Layton <jlayton@...nel.org>,
	Steve French <smfrench@...il.com>,
	Matthew Wilcox <willy@...radead.org>,
	Marc Dionne <marc.dionne@...istor.com>,
	Paulo Alcantara <pc@...guebit.com>,
	Shyam Prasad N <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>,
	Dominique Martinet <asmadeus@...ewreck.org>,
	Eric Van Hensbergen <ericvh@...nel.org>,
	Ilya Dryomov <idryomov@...il.com>,
	Christian Brauner <christian@...uner.io>, linux-cachefs@...hat.com,
	linux-afs@...ts.infradead.org, linux-cifs@...r.kernel.org,
	linux-nfs@...r.kernel.org, ceph-devel@...r.kernel.org,
	v9fs@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
	linux-mm@...ck.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
	Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v5 15/40] netfs: Add support for DIO buffering

On Thu, Dec 28, 2023 at 11:47:45AM +0100, Christian Brauner wrote:
> > This will break the build with versions of clang that have support for
> > counted_by (as it has been reverted in main but reapplication to main is
> > being actively worked on) because while annotating pointers with this
> > attribute is a goal of the counted_by attribute, it is not ready yet.
> > Please consider removing this and adding a TODO to annotate it when
> > support is available.
> 
> It's really unpleasant that we keep getting new attributes that we
> seemingly are encouraged to use and get sent patches for it. And then we
> learn a little later that that stuff isn't ready yet. It's annoying. I

I will assume the "get sent patches for it" is referring to the patches
that Kees has been sending out to add this attribute to flexible array
members. In his defense, that part of the attribute is very nearly ready
(it is only the pointer annotations that are not ready, as in not worked
on at all as far as I am aware). In fact, it was merged in clang's main
branch for some time and the only reason that it was backed out was
because adoption in the kernel had pointed out bugs in the original
implementation that were harder to fix than initially thought; in other
words, only because we started adding this attribute to the kernel were
we able to realize that the initial implementation in clang needed to be
improved, otherwise this feature may have shipped completely broken in
clang 18.1.0 because it had not been stress tested yet. Now we can get
it right.

However, I do not necessarily disagree that it is annoying for
maintainers who are not following this saga but are just receiving
patches to add these annotatations because adds additional things to
check for. Perhaps there should be some guidance added to the
__counted_by definition or Documentation around how it is expected to be
used so that there is clear advice for both developers and maintainers?

> know it isn't your fault but it would be wise to be a little more
> careful. IOW, unless both clang and gcc do support that thing
> appropriately don't send patches to various subsystems for this.

I will assume this was not necessarily directed at me because I have not
sent any patches for __counted_by.

> In any case, this is now fixed. I pulled an updated version from David.

Thanks a lot.

Cheers,
Nathan