| Message ID | 20241202145946.108093528@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | module: Strict per-modname namespaces | expand |
On Mon, Dec 02, 2024 at 03:59:47PM +0100, Peter Zijlstra wrote: > 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. > > git grep -l -e MODULE_IMPORT_NS -e EXPORT_SYMBOL_NS | while read file; > do > awk -i inplace ' > /^#define EXPORT_SYMBOL_NS/ { > gsub(/__stringify\(ns\)/, "ns"); > print; > next; > } > /^#define MODULE_IMPORT_NS/ { > gsub(/__stringify\(ns\)/, "ns"); > print; > next; > } > /MODULE_IMPORT_NS/ { > $0 = gensub(/MODULE_IMPORT_NS\(([^)]*)\)/, "MODULE_IMPORT_NS(\"\\1\")", "g"); > } > /EXPORT_SYMBOL_NS/ { > if ($0 ~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+),/) { > if ($0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/ && > $0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(\)/ && > $0 !~ /^my/) { > getline line; > gsub(/[[:space:]]*\\$/, ""); > gsub(/[[:space:]]/, "", line); > $0 = $0 " " line; > } > > $0 = gensub(/(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/, > "\\1(\\2, \"\\3\")", "g"); > } > } > { print }' $file; > done Perhaps we can ask Linus to run this now, before -next fills up again ? > Requested-by: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
On Mon, Dec 02, 2024 at 04:15:33PM +0100, Peter Zijlstra wrote: > On Mon, Dec 02, 2024 at 03:59:47PM +0100, Peter Zijlstra wrote: > > 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. > > > > git grep -l -e MODULE_IMPORT_NS -e EXPORT_SYMBOL_NS | while read file; > > do > > awk -i inplace ' > > /^#define EXPORT_SYMBOL_NS/ { > > gsub(/__stringify\(ns\)/, "ns"); > > print; > > next; > > } > > /^#define MODULE_IMPORT_NS/ { > > gsub(/__stringify\(ns\)/, "ns"); > > print; > > next; > > } > > /MODULE_IMPORT_NS/ { > > $0 = gensub(/MODULE_IMPORT_NS\(([^)]*)\)/, "MODULE_IMPORT_NS(\"\\1\")", "g"); > > } > > /EXPORT_SYMBOL_NS/ { > > if ($0 ~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+),/) { > > if ($0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/ && > > $0 !~ /(EXPORT_SYMBOL_NS[^(]*)\(\)/ && > > $0 !~ /^my/) { > > getline line; > > gsub(/[[:space:]]*\\$/, ""); > > gsub(/[[:space:]]/, "", line); > > $0 = $0 " " line; > > } > > > > $0 = gensub(/(EXPORT_SYMBOL_NS[^(]*)\(([^,]+), ([^)]+)\)/, > > "\\1(\\2, \"\\3\")", "g"); > > } > > } > > { print }' $file; > > done > > Perhaps we can ask Linus to run this now, before -next fills up again ? Yes please!
Peter Zijlstra <peterz@infradead.org> writes: > 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. Blast from the past: https://lists.linuxcoding.com/kernel/2007-q4/msg19926.html Yes it makes sense. -Andi
On Mon, 2 Dec 2024 at 07:15, Peter Zijlstra <peterz@infradead.org> wrote: > > Perhaps we can ask Linus to run this now, before -next fills up again ? Sure. I did an unasked-for scripted 'remove_new' removal right after rc1 for the same reason. If we have these kinds of big scripted things, right after the merge window tends to be the best time to do them. The conflict potential of leaving it hanging in linux-next can be somewhat annoying. They may be fairly unlikely, and easy to resolve individually, but it's one of those "one is trivial to deal with, but even just a handful is annoying". So I'll run your script and take your commit message, and we'll have this part over and done with. Thanks, Linus
On Mon, Dec 02, 2024 at 11:33:58AM -0800, Linus Torvalds wrote: > On Mon, 2 Dec 2024 at 07:15, Peter Zijlstra <peterz@infradead.org> wrote: > > > > Perhaps we can ask Linus to run this now, before -next fills up again ? > > Sure. I did an unasked-for scripted 'remove_new' removal right after > rc1 for the same reason. > > If we have these kinds of big scripted things, right after the merge > window tends to be the best time to do them. The conflict potential of > leaving it hanging in linux-next can be somewhat annoying. They may be > fairly unlikely, and easy to resolve individually, but it's one of > those "one is trivial to deal with, but even just a handful is > annoying". > > So I'll run your script and take your commit message, and we'll have > this part over and done with. I *think* this is interacting in a fun way with at least the IIO subsystem in -next (Linus' tree is fine, I didn't do too much investigation as I'd quite like the -next build to finish some time today): /tmp/next/tmp/ccjk7YJR.s: Assembler messages: /tmp/next/tmp/ccjk7YJR.s:5: Error: junk at end of line, first unrecognized character is `I' make[7]: *** [/tmp/next/build/scripts/Makefile.build:194: drivers/iio/imu/inv_icm42600/inv_icm42600_core.o] Error 1 I've dropped the IIO trees from -next for now.
On Tue, Dec 03, 2024 at 07:20:05PM +0000, Mark Brown wrote: > On Mon, Dec 02, 2024 at 11:33:58AM -0800, Linus Torvalds wrote: > > If we have these kinds of big scripted things, right after the merge > > window tends to be the best time to do them. The conflict potential of > > leaving it hanging in linux-next can be somewhat annoying. They may be > > fairly unlikely, and easy to resolve individually, but it's one of > > those "one is trivial to deal with, but even just a handful is > > annoying". > > So I'll run your script and take your commit message, and we'll have > > this part over and done with. > I *think* this is interacting in a fun way with at least the IIO > subsystem in -next (Linus' tree is fine, I didn't do too much > investigation as I'd quite like the -next build to finish some time > today): Yes, this is breaking ASoC and possibly other things as well. I guess any tree adding a new use of these macros needs to merge mainline to avoid a mess here.
On Wed, Dec 4, 2024 at 7:06 AM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Dec 03, 2024 at 07:20:05PM +0000, Mark Brown wrote: > > On Mon, Dec 02, 2024 at 11:33:58AM -0800, Linus Torvalds wrote: > > > > If we have these kinds of big scripted things, right after the merge > > > window tends to be the best time to do them. The conflict potential of > > > leaving it hanging in linux-next can be somewhat annoying. They may be > > > fairly unlikely, and easy to resolve individually, but it's one of > > > those "one is trivial to deal with, but even just a handful is > > > annoying". > > > > So I'll run your script and take your commit message, and we'll have > > > this part over and done with. > > > I *think* this is interacting in a fun way with at least the IIO > > subsystem in -next (Linus' tree is fine, I didn't do too much > > investigation as I'd quite like the -next build to finish some time > > today): > > Yes, this is breaking ASoC and possibly other things as well. I guess > any tree adding a new use of these macros needs to merge mainline to > avoid a mess here. In this development cycle, I think subsystems should queue up patches on top of -rc2 instead of -rc1. Meanwhile, linux-next needs to carry the following fixup. diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c index 550cc53e7559..f415f300afb6 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c @@ -908,4 +908,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = { MODULE_AUTHOR("InvenSense, Inc."); MODULE_DESCRIPTION("InvenSense ICM-426xx device driver"); MODULE_LICENSE("GPL"); -MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP); +MODULE_IMPORT_NS("IIO_INV_SENSORS_TIMESTAMP");
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 'patch' is an awk scripts that cleans up the existing module namespace code along the same lines of commit 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. 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 With that, some configs generate: ERROR: modpost: module kvmgt uses symbol kvm_write_track_add_gfn from namespace MODULE_kvm,kvm-intel,kvm-amd, but does not import it. ERROR: modpost: module kvmgt uses symbol kvm_write_track_remove_gfn from namespace MODULE_kvm,kvm-intel,kvm-amd, but does not import it. ERROR: modpost: module kvmgt uses symbol kvm_page_track_register_notifier from namespace MODULE_kvm,kvm-intel,kvm-amd, but does not import it. ERROR: modpost: module kvmgt uses symbol kvm_page_track_unregister_notifier from namespace MODULE_kvm,kvm-intel,kvm-amd, but does not import it. Showing it works :-). Also verified that once booted, the module kvm_intel does actually load. Also available at: git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git module/namespace Changes since RFC/v1: - use awk instead of sed so all changes are a single script (hch) - deal with kbuild mangling the module names like s/-/_/g (sean) - fixup clang-ias 'funnies'