Message-ID: <20250625-blatt-lieblich-8e6896fe618b@brauner>
Date: Wed, 25 Jun 2025 11:05:50 +0200
From: Christian Brauner <brauner@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Vlastimil Babka <vbabka@...e.cz>, 
	Christoph Hellwig <hch@...radead.org>, Sean Christopherson <seanjc@...gle.com>, 
	Mike Rapoport <rppt@...nel.org>, Shivank Garg <shivankg@....com>, david@...hat.com, 
	akpm@...ux-foundation.org, paul@...l-moore.com, viro@...iv.linux.org.uk, 
	willy@...radead.org, pbonzini@...hat.com, tabba@...gle.com, afranji@...gle.com, 
	ackerleytng@...gle.com, jack@...e.cz, cgzones@...glemail.com, ira.weiny@...el.com, 
	roypat@...zon.co.uk, linux-fsdevel@...r.kernel.org, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH] fs: export anon_inode_make_secure_inode() and fix
 secretmem LSM bypass

On Tue, Jun 24, 2025 at 11:02:16AM +0200, Christian Brauner wrote:
> On Mon, Jun 23, 2025 at 04:28:36PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 23, 2025 at 04:21:15PM +0200, Vlastimil Babka wrote:
> > > On 6/23/25 16:01, Christoph Hellwig wrote:
> > > > On Mon, Jun 23, 2025 at 07:00:39AM -0700, Christoph Hellwig wrote:
> > > >> On Mon, Jun 23, 2025 at 12:16:27PM +0200, Christian Brauner wrote:
> > > >> > I'm more than happy to switch a bunch of our exports so that we only
> > > >> > allow them for specific modules. But for that we also need
> > > >> > EXPOR_SYMBOL_FOR_MODULES() so we can switch our non-gpl versions.
> > > >> 
> > > >> Huh?  Any export for a specific in-tree module (or set thereof) is
> > > >> by definition internals and an _GPL export if perfectly fine and
> > > >> expected.
> > > 
> > > Peterz tells me EXPORT_SYMBOL_GPL_FOR_MODULES() is not limited to in-tree
> > > modules, so external module with GPL and matching name can import.
> > > 
> > > But if we're targetting in-tree stuff like kvm, we don't need to provide a
> > > non-GPL variant I think?
> > 
> > So the purpose was to limit specific symbols to known in-tree module
> > users (hence GPL only).
> > 
> > Eg. KVM; x86 exports a fair amount of low level stuff just because KVM.
> > Nobody else should be touching those symbols.
> > 
> > If you have a pile of symbols for !GPL / out-of-tree consumers, it
> > doesn't really make sense to limit the export to a named set of modules,
> > does it?

I don't understand that argument. I don't care if out-of-tree users
abuse some symbol because:

* If they ever show up with a complaint we'll tell them to go away. 
* If they want to be merged upstream, we'll tell them to either change
  the code in question to not rely on the offending symbol or we decide
  that it's ok for them to use it and allow-list them.

I do however very much care about in-tree consumers even for non-GPLd
symbols. I want anyone who tries to use a symbol that we decided
requires substantial arguments to be used to come to us and justify it.
So EXPORT_*_FOR_MODULES() would certainly help with that.

The other things is that using EXPORT_SYMBOL() or even
EXPORT_SYMBOL_GPL() sends the wrong message to other module-like
wanna-be consumers of these functions. I'm specifically thinking about
bpf. They more than once argued that anything exposed to modules can
also be exposed as a bpf kfunc as the stability guarantees are
comparable.

And it is not an insane argument. Being able to use
EXPOR_SYMBOL_FOR_MODULES() would also allow to communicate "Hey, this
very much just exists for the purpose of this one-off consumer that
really can't do without it or without some other ugly hack.".

Because this is where the pain for us is: If you do large-scale
refactorings (/me glares at Al, Christoph, and in the mirror) the worst
case is finding out that some special-purpose helper has grown N new
users with a bunch of them using it completely wrong and now having to
massage core code to not break something that's technically inherently
broken.

Out-of-tree consumers have zero relevance for all of this. For all I
care they don't exist. It's about the maintainers ability to chop off
the Kraken's tentacles.

> > 
> > So yes, nothing limits things to in-tree modules per-se. The
> > infrastructure only really cares about module names (and implicitly
> > trusts the OS to not overwrite existing kernel modules etc.). So you
> > could add an out-of-tree module name to the list (or have an out-of-free
> > module have a name that matches a glob; "kvm-vmware" would match "kvm-*"
> > for example).
> > 
> > But that is very much beyond the intention of things.
> 
> So I'm not well-versed in all the GPL vs non-GPL exports. I'm thinking
> of cases like EXPORT_SYMBOL(fget_task_next); That's exposed to gfs2 (and
> bpf but that's built-in). I see no reason to risk spreading the usage of
> that special-thing to anywhere else. So I would use
> EXPORT_*_FOR_MODULES(gfs2) for this and we'd notice if anything else is
> trying to use that thing.
> 
> Another excellent candidate is:
> 
>   /*
>    * synchronous analog of fput(); for kernel threads that might be needed
>    * in some umount() (and thus can't use flush_delayed_fput() without
>    * risking deadlocks), need to wait for completion of __fput() and know
>    * for this specific struct file it won't involve anything that would
>    * need them.  Use only if you really need it - at the very least,
>    * don't blindly convert fput() by kernel thread to that.
>    */
>   void __fput_sync(struct file *file)
>   {
>           if (file_ref_put(&file->f_ref))
>                   __fput(file);
>   }
>   EXPORT_SYMBOL(__fput_sync);
> 
> That thing worries me to no end because that can be used to wreak all
> kinds of havoc and I want that thing tied down so no one can even look
> at it without getting a compile time or runtime error that we can
> immediately notice. So for that as well I want to allow-list modules
> that we have explictly acknowledged to use it.
> 
> But iiuc I can't just switch that non-GPL exported symbol to a GPL
> exported symbol. And I don't want to be involved in some kind of
> ideological warfare around that stuff.
> 
> I care about not growing more users of __fput_sync(). So any advice is
> appreciated.

s/./?/

That was supposed to be a question mark.

IOW, can I add EXPORT_SYMBOL_FOR_MODULES() as a companion to
EXPORT_SYMBOL_GPL_FOR_MODULES()?

Powered by blists - more mailing lists