On Tue, Nov 12, 2024, Peter Zijlstra wrote:
> On Mon, Nov 11, 2024 at 04:48:58PM -0800, Sean Christopherson wrote:
> > On Mon, Nov 11, 2024, Peter Zijlstra wrote:
> > > Hi!
> > >
> > > Implement a means for exports to be available only to an explicit list of named
> > > modules. By explicitly limiting the usage of certain exports, the abuse
> > > potential/risk is greatly reduced.
> > >
> > > The first three 'patches' clean up the existing export namespace code along the
> > > same lines of 33def8498fdd ("treewide: Convert macro and uses of __section(foo)
> > > to __section("foo")") and for the same reason, it is not desired for the
> > > namespace argument to be a macro expansion itself.
> > >
> > > In fact, the second patch is really only a script, because sending the output
> > > to the list is a giant waste of bandwidth. Whoever eventually commits this to a
> > > git tree should squash these first three patches.
> > >
> > > The remainder of the patches introduce the special "MODULE_<modname-list>"
> > > namespace, which shall be forbidden from being explicitly imported. A module
> > > that matches the simple modname-list will get an implicit import.
> > >
> > > Lightly tested with something like:
> > >
> > > git grep -l EXPORT_SYMBOL arch/x86/kvm/ | while read file;
> > > do
> > > sed -i -e 's/EXPORT_SYMBOL_GPL(\(.[^)]*\))/EXPORT_SYMBOL_GPL_FOR(\1, "kvm,kvm-intel,kvm-amd")/g' $file;
> > > done
> >
> > Heh, darn modules. This will compile just fine, but if the module contains a
> > dash, loading the module will fail because scripts/Makefile.lib replaces the dash
> > with an underscore the build name. E.g. "kvm-intel" at compile time generates
> > kvm-intel.ko, but the actual name of the module as seen by the kernel is kvm_intel.
>
> I was wondering about that... WTH is kvm doing that?
No idea. The naming has been that way since KVM's inception in commit 6aa8b732ca01
("[PATCH] kvm: userspace interface"). My guess is that either no one noticed, or
those who noticed didn't care.
FWIW, IMO the kernel build system is the one that's being weird. AFAICT, the
'-' => '_' conversion was added so that spinlocks could be placed into unique
subsections. Amusingly, it doesn't appear that there are any remaining users of
LOCK_SECTION_NAME.
commit b5635319d32438ed516568f53013a460ba16e6ee
Author: Dave Jones <davej@xxxxxxx>
AuthorDate: Fri Feb 8 01:43:23 2002 -0800
Commit: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxxx>
CommitDate: Fri Feb 8 01:43:23 2002 -0800
[PATCH] text.lock -> subsection changes.
Make spinlocks etc use subsections of their parent sections instead of
an ELF section of their own - needed for newer binutils when the parent
sector is removed.
#define LOCK_SECTION_NAME ".text..lock."KBUILD_BASENAME
#define LOCK_SECTION_START(extra) \
".subsection 1\n\t" \
extra \
".ifndef " LOCK_SECTION_NAME "\n\t" \
LOCK_SECTION_NAME ":\n\t" \
".endif\n"
#define LOCK_SECTION_END \
".previous\n\t"
#define __lockfunc __section(".spinlock.text")
> I mean, I suppose you can do: "kvm-intel,kvm_intel" but that's somewhat
> tedious.
This likely needs to be addressed in whatever chunk of code is enforcing the
namespaces. The s/-/_ behavior (and vice versa!) is *very* baked into the kernel
at this point, e.g. parameqn() will happily parse dashes or underscores for every
kernel parameter. As horrific as it is, I think the module namespace needs to do
the same, i.e. treat dashes and underscores as one and the same.
More historical amusement:
commit 8863179c65618844379ef90d4a708293042465c8
Author: Andrew Morton <akpm@xxxxxxxxx>
AuthorDate: Sun Feb 2 06:08:27 2003 -0800
Commit: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxx>
CommitDate: Sun Feb 2 06:08:27 2003 -0800
[PATCH] kernel param and KBUILD_MODNAME name-munging mess
Patch from: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Mikael Pettersson points out that "-s" gets mangled to "_s" on the
kernel command line, even though it turns out not to be a
parameter.
commit 326e7842d30d5cfc1089b85a7aa63e5c9f3c0a74
Author: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
AuthorDate: Sat Dec 14 20:13:11 2002 -0800
Commit: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxx>
CommitDate: Sat Dec 14 20:13:11 2002 -0800
[PATCH] Module Parameter Core Patch
This patch is a rewrite of the insmod and boot parameter handling,
to unify them.
The new format is fairly simple: built on top of __module_param_call there
are several helpers, eg "module_param(foo, int, 000)". The final argument
is the permissions bits, for exposing parameters in sysfs (if
non-zero) at a later stage.
