| Message ID | 20241111105430.575636482@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | module: Strict per-modname namespaces | expand |
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. -- # These flags are needed for modversions and compiling, so we define them here # $(modname_flags) defines KBUILD_MODNAME as the name of the module it will # end up in (or would, if it gets compiled in) name-fix-token = $(subst $(comma),_,$(subst -,_,$1)) <==================== name-fix = $(call stringify,$(call name-fix-token,$1)) basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \ -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname)) modfile_flags = -DKBUILD_MODFILE=$(call stringify,$(modfile)) --
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? I mean, I suppose you can do: "kvm-intel,kvm_intel" but that's somewhat tedious.
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@suse.de> AuthorDate: Fri Feb 8 01:43:23 2002 -0800 Commit: Linus Torvalds <torvalds@penguin.transmeta.com> 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@digeo.com> AuthorDate: Sun Feb 2 06:08:27 2003 -0800 Commit: Linus Torvalds <torvalds@home.transmeta.com> CommitDate: Sun Feb 2 06:08:27 2003 -0800 [PATCH] kernel param and KBUILD_MODNAME name-munging mess Patch from: Rusty Russell <rusty@rustcorp.com.au> 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@rustcorp.com.au> AuthorDate: Sat Dec 14 20:13:11 2002 -0800 Commit: Linus Torvalds <torvalds@home.transmeta.com> 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.
On Tue, Nov 12, 2024 at 09:56:20AM -0800, Sean Christopherson wrote: > 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. Right, I'll add a s/-/_/g on both ends of the strcmp or somesuch.
On Tue, Nov 12, 2024 at 08:52:26PM +0100, Peter Zijlstra wrote: > On Tue, Nov 12, 2024 at 09:56:20AM -0800, Sean Christopherson wrote: > > > 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. > > Right, I'll add a s/-/_/g on both ends of the strcmp or somesuch. https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=module/namespace&id=f2aabf8436348a47037570af139ec2c1de8c5337 My test box seems able to load kvm_intel. I'll let it all sit in that tree for a few days such that 0day might get a chance to chew on it before posting. Thanks!
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 Also available at: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git module/namespace