| Message ID | 20220130213214.1042497-1-atomlin@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | module: core code clean up | expand |
Thanks Aaron for V4. Build/boot test on qemu. Running more tests on BM. Thanks. On Sun, Jan 30, 2022 at 1:32 PM Aaron Tomlin <atomlin@redhat.com> wrote: > > Hi Luis, > > As per your suggestion [1], this is an attempt to refactor and split > optional code out of core module support code into separate components. > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > modules-5.17-rc1. Please let me know your thoughts. > > Changes since v1 [2]: > > - Moved module version support code into a new file > > Changes since v2 [3]: > > - Moved module decompress support to a separate file > - Made check_modinfo_livepatch() generic (Petr Mladek) > - Removed filename from each newly created file (Luis Chamberlain) > - Addressed some (i.e. --ignore=ASSIGN_IN_IF,AVOID_BUG was used) > minor scripts/checkpatch.pl concerns e.g., use strscpy over > strlcpy and missing a blank line after declarations (Allen) > > Changes since v3 [4]: > > - Refactored both is_livepatch_module() and set_livepatch_module(), > respectively, to use IS_ENABLED(CONFIG_LIVEPATCH) (Joe Perches) > - Addressed various compiler warnings e.g., no previous prototype (0-day) > > [1]: https://lore.kernel.org/lkml/YbEZ4HgSYQEPuRmS@bombadil.infradead.org/ > [2]: https://lore.kernel.org/lkml/20211228213041.1356334-1-atomlin@redhat.com/ > [3]: https://lore.kernel.org/lkml/20220106234319.2067842-1-atomlin@redhat.com/ > [4]: https://lore.kernel.org/lkml/20220128203934.600247-1-atomlin@redhat.com/ > > Aaron Tomlin (13): > module: Move all into module/ > module: Simple refactor in preparation for split > module: Move livepatch support to a separate file > module: Move latched RB-tree support to a separate file > module: Move arch strict rwx support to a separate file > module: Move strict rwx support to a separate file > module: Move extra signature support out of core code > module: Move kmemleak support to a separate file > module: Move kallsyms support into a separate file > module: Move procfs support into a separate file > module: Move sysfs support into a separate file > module: Move kdb_modules list out of core code > module: Move version support into a separate file > > MAINTAINERS | 2 +- > include/linux/module.h | 64 +- > kernel/Makefile | 5 +- > kernel/debug/kdb/kdb_main.c | 5 + > kernel/module-internal.h | 50 - > kernel/module/Makefile | 20 + > kernel/module/arch_strict_rwx.c | 44 + > kernel/module/debug_kmemleak.c | 30 + > .../decompress.c} | 2 +- > kernel/module/internal.h | 236 +++ > kernel/module/kallsyms.c | 502 +++++ > kernel/module/livepatch.c | 74 + > kernel/{module.c => module/main.c} | 1874 +---------------- > kernel/module/procfs.c | 142 ++ > .../signature.c} | 0 > kernel/module/signing.c | 120 ++ > kernel/module/strict_rwx.c | 83 + > kernel/module/sysfs.c | 425 ++++ > kernel/module/tree_lookup.c | 109 + > kernel/module/version.c | 110 + > kernel/module_signing.c | 45 - > 21 files changed, 2038 insertions(+), 1904 deletions(-) > delete mode 100644 kernel/module-internal.h > create mode 100644 kernel/module/Makefile > create mode 100644 kernel/module/arch_strict_rwx.c > create mode 100644 kernel/module/debug_kmemleak.c > rename kernel/{module_decompress.c => module/decompress.c} (99%) > create mode 100644 kernel/module/internal.h > create mode 100644 kernel/module/kallsyms.c > create mode 100644 kernel/module/livepatch.c > rename kernel/{module.c => module/main.c} (63%) > create mode 100644 kernel/module/procfs.c > rename kernel/{module_signature.c => module/signature.c} (100%) > create mode 100644 kernel/module/signing.c > create mode 100644 kernel/module/strict_rwx.c > create mode 100644 kernel/module/sysfs.c > create mode 100644 kernel/module/tree_lookup.c > create mode 100644 kernel/module/version.c > delete mode 100644 kernel/module_signing.c > > > base-commit: a97ac8cb24a3c3ad74794adb83717ef1605d1b47 > -- > 2.34.1 >
On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: > Hi Luis, > > As per your suggestion [1], this is an attempt to refactor and split > optional code out of core module support code into separate components. > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > modules-5.17-rc1. Please let me know your thoughts. Nice, can you get this tested with 0day? You can ask for your tree to be tested, see: https://01.org/lkp/documentation/0-day-test-service See the question, "Which git tree and which mailing list will be tested? How can I opt-in or opt-out from it?" LUis
On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: > Hi Luis, > > As per your suggestion [1], this is an attempt to refactor and split > optional code out of core module support code into separate components. > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > modules-5.17-rc1. Please let me know your thoughts. > > Changes since v1 [2]: Thanks for all this work Aaron! Can you drop the RFC prefix, rebase onto linus' latest tree (as he already merged my modules-next, so his tree is more up to date), and submit again? I'll then apply this to my modules-next, and then ask Christophe to rebase on top of that. Michal, you'd be up next if you want to go through modules-next. Aaron, please Cc Christophe and Michal on your next respin. Thanks!! Luis
Le 03/02/2022 à 01:20, Luis Chamberlain a écrit : > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: >> Hi Luis, >> >> As per your suggestion [1], this is an attempt to refactor and split >> optional code out of core module support code into separate components. >> This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or >> modules-5.17-rc1. Please let me know your thoughts. >> >> Changes since v1 [2]: > > Thanks for all this work Aaron! Can you drop the RFC prefix, > rebase onto linus' latest tree (as he already merged my > modules-next, so his tree is more up to date), and submit again? > > I'll then apply this to my modules-next, and then ask Christophe to > rebase on top of that. > > Michal, you'd be up next if you want to go through modules-next. > > Aaron, please Cc Christophe and Michal on your next respin. > I went quickly through v4. That's a great and useful job I think, it should ease future work on modules. So I will be happy to apply my changes on top of this series. I may have more comments after reviewing in more details and rebasing my series on top of it, but at the time being I have at least one comment: All function prototypes in header files are pointlessly prepended with 'extern' keyword. This was done that way in the old days, but has been deprecated as this keyword does nothing on function prototypes but adds visual pollution when looking at the files. See below the output of checkpatch on internal.h Regardless, I'm looking forward to rebasing my series on top of yours. Thanks Christophe WARNING: Use #include <linux/module.h> instead of <asm/module.h> #10: FILE: kernel/module/internal.h:10: +#include <asm/module.h> CHECK: spaces preferred around that '-' (ctx:VxV) #18: FILE: kernel/module/internal.h:18: +#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) ^ CHECK: extern prototypes should be avoided in .h files #84: FILE: kernel/module/internal.h:84: +extern int mod_verify_sig(const void *mod, struct load_info *info); CHECK: extern prototypes should be avoided in .h files #85: FILE: kernel/module/internal.h:85: +extern int try_to_force_load(struct module *mod, const char *reason); CHECK: extern prototypes should be avoided in .h files #86: FILE: kernel/module/internal.h:86: +extern bool find_symbol(struct find_symbol_arg *fsa); CHECK: extern prototypes should be avoided in .h files #87: FILE: kernel/module/internal.h:87: +extern struct module *find_module_all(const char *name, size_t len, bool even_unformed); CHECK: extern prototypes should be avoided in .h files #88: FILE: kernel/module/internal.h:88: +extern unsigned long kernel_symbol_value(const struct kernel_symbol *sym); CHECK: extern prototypes should be avoided in .h files #89: FILE: kernel/module/internal.h:89: +extern int cmp_name(const void *name, const void *sym); CHECK: extern prototypes should be avoided in .h files #90: FILE: kernel/module/internal.h:90: +extern long get_offset(struct module *mod, unsigned int *size, Elf_Shdr *sechdr, CHECK: extern prototypes should be avoided in .h files #92: FILE: kernel/module/internal.h:92: +extern char *module_flags(struct module *mod, char *buf); CHECK: extern prototypes should be avoided in .h files #95: FILE: kernel/module/internal.h:95: +extern int copy_module_elf(struct module *mod, struct load_info *info); CHECK: extern prototypes should be avoided in .h files #96: FILE: kernel/module/internal.h:96: +extern void free_module_elf(struct module *mod); CHECK: Please use a blank line after function/struct/union/enum declarations #102: FILE: kernel/module/internal.h:102: +} +static inline void free_module_elf(struct module *mod) { } CHECK: Please use a blank line after function/struct/union/enum declarations #114: FILE: kernel/module/internal.h:114: +} +static inline void module_decompress_cleanup(struct load_info *info) CHECK: extern prototypes should be avoided in .h files #128: FILE: kernel/module/internal.h:128: +extern void mod_tree_insert(struct module *mod); CHECK: extern prototypes should be avoided in .h files #129: FILE: kernel/module/internal.h:129: +extern void mod_tree_remove_init(struct module *mod); CHECK: extern prototypes should be avoided in .h files #130: FILE: kernel/module/internal.h:130: +extern void mod_tree_remove(struct module *mod); CHECK: extern prototypes should be avoided in .h files #131: FILE: kernel/module/internal.h:131: +extern struct module *mod_find(unsigned long addr); ERROR: do not initialise statics to 0 #133: FILE: kernel/module/internal.h:133: +static unsigned long module_addr_min = -1UL, module_addr_max = 0; CHECK: extern prototypes should be avoided in .h files #153: FILE: kernel/module/internal.h:153: +extern int module_sig_check(struct load_info *info, int flags); CHECK: extern prototypes should be avoided in .h files #162: FILE: kernel/module/internal.h:162: +extern void kmemleak_load_module(const struct module *mod, const struct load_info *info); CHECK: extern prototypes should be avoided in .h files #170: FILE: kernel/module/internal.h:170: +extern void init_build_id(struct module *mod, const struct load_info *info); CHECK: extern prototypes should be avoided in .h files #175: FILE: kernel/module/internal.h:175: +extern void layout_symtab(struct module *mod, struct load_info *info); CHECK: extern prototypes should be avoided in .h files #176: FILE: kernel/module/internal.h:176: +extern void add_kallsyms(struct module *mod, const struct load_info *info); CHECK: extern prototypes should be avoided in .h files #177: FILE: kernel/module/internal.h:177: +extern bool sect_empty(const Elf_Shdr *sect); CHECK: extern prototypes should be avoided in .h files #178: FILE: kernel/module/internal.h:178: +extern const char *find_kallsyms_symbol(struct module *mod, unsigned long addr, CHECK: extern prototypes should be avoided in .h files #191: FILE: kernel/module/internal.h:191: +extern int mod_sysfs_setup(struct module *mod, const struct load_info *info, CHECK: extern prototypes should be avoided in .h files #193: FILE: kernel/module/internal.h:193: +extern void mod_sysfs_fini(struct module *mod); CHECK: extern prototypes should be avoided in .h files #194: FILE: kernel/module/internal.h:194: +extern void module_remove_modinfo_attrs(struct module *mod, int end); CHECK: extern prototypes should be avoided in .h files #195: FILE: kernel/module/internal.h:195: +extern void del_usage_links(struct module *mod); CHECK: extern prototypes should be avoided in .h files #196: FILE: kernel/module/internal.h:196: +extern void init_param_lock(struct module *mod); CHECK: Please use a blank line after function/struct/union/enum declarations #205: FILE: kernel/module/internal.h:205: +} +static inline void mod_sysfs_fini(struct module *mod) { } CHECK: extern prototypes should be avoided in .h files #212: FILE: kernel/module/internal.h:212: +extern int check_version(const struct load_info *info, CHECK: extern prototypes should be avoided in .h files #214: FILE: kernel/module/internal.h:214: +extern int check_modstruct_version(const struct load_info *info, struct module *mod); CHECK: extern prototypes should be avoided in .h files #215: FILE: kernel/module/internal.h:215: +extern int same_magic(const char *amagic, const char *bmagic, bool has_crcs); CHECK: Alignment should match open parenthesis #232: FILE: kernel/module/internal.h:232: +static inline int same_magic(const char *amagic, const char *bmagic, + bool has_crcs) total: 1 errors, 1 warnings, 34 checks, 236 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. kernel/module/internal.h has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
Le 03/02/2022 à 01:20, Luis Chamberlain a écrit : > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: >> Hi Luis, >> >> As per your suggestion [1], this is an attempt to refactor and split >> optional code out of core module support code into separate components. >> This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or >> modules-5.17-rc1. Please let me know your thoughts. >> >> Changes since v1 [2]: > I have another comment: I think patch 5 should be dropped. Having something behave based on a CONFIG_ARCH_HAS_SOMETHING item is wrong. It is not because a plateform selects CONFIG_ARCH_HAS_STRICT_MODULE_RWX that the module core should behave differentely than with other platforms as far as the user has not selected CONFIG_STRICT_MODULE_RWX. And the topic here is wrong. It is a coincidence if making that stuff depend on CONFIG_ARCH_HAS_STRICT_MODULE_RWX works. This is just because the only architectures that do the module allocation without Exec flag are architectures that have also selected CONFIG_ARCH_HAS_STRICT_MODULE_RWX. But it should also work on other architectures. I don't know exactly what was the motivation for commit 93651f80dcb6 ("modules: fix compile error if don't have strict module rwx") at the first place but it is just wrong and we should fix it. module_enable_x() should work just fine regardless of CONFIG_ARCH_HAS_STRICT_MODULE_RWX. Thanks Christophe
Le 03/02/2022 à 01:20, Luis Chamberlain a écrit : > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: >> Hi Luis, >> >> As per your suggestion [1], this is an attempt to refactor and split >> optional code out of core module support code into separate components. >> This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or >> modules-5.17-rc1. Please let me know your thoughts. >> I also have the feeling that a lot of stuff like function prototypes you added in include/linux/module.h should instead go in kernel/module/internal.h as they are not used outside of kernel/module/ Christophe
Hello, On Wed, Feb 02, 2022 at 04:20:41PM -0800, Luis Chamberlain wrote: > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: > > Hi Luis, > > > > As per your suggestion [1], this is an attempt to refactor and split > > optional code out of core module support code into separate components. > > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > > modules-5.17-rc1. Please let me know your thoughts. > > > > Changes since v1 [2]: > > Thanks for all this work Aaron! Can you drop the RFC prefix, > rebase onto linus' latest tree (as he already merged my > modules-next, so his tree is more up to date), and submit again? > > I'll then apply this to my modules-next, and then ask Christophe to > rebase on top of that. > > Michal, you'd be up next if you want to go through modules-next. Sounds like a good idea. When rebasing on top of 5.17-rc1 the only conflict was on the module code. Thanks Michal
On Wed, Feb 02, 2022 at 04:20:41PM -0800, Luis Chamberlain wrote: > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: > > Hi Luis, > > > > As per your suggestion [1], this is an attempt to refactor and split > > optional code out of core module support code into separate components. > > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > > modules-5.17-rc1. Please let me know your thoughts. > > > > Changes since v1 [2]: > > Thanks for all this work Aaron! Can you drop the RFC prefix, > rebase onto linus' latest tree (as he already merged my > modules-next, so his tree is more up to date), and submit again? Linus now merged the fix in question, just be sure to use his latest tree, it should include 67d6212afda218d564890d1674bab28e8612170f > I'll then apply this to my modules-next, and then ask Christophe to > rebase on top of that. If you can fix the issues from your patches which Christophe mentioned that would be great. Then I'll apply then and then Christophe can work off of that. > Michal, you'd be up next if you want to go through modules-next. Luis
On Thu, Feb 03, 2022 at 08:43:17PM +0100, Michal Suchánek wrote: > Hello, > > On Wed, Feb 02, 2022 at 04:20:41PM -0800, Luis Chamberlain wrote: > > On Sun, Jan 30, 2022 at 09:32:01PM +0000, Aaron Tomlin wrote: > > > Hi Luis, > > > > > > As per your suggestion [1], this is an attempt to refactor and split > > > optional code out of core module support code into separate components. > > > This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or > > > modules-5.17-rc1. Please let me know your thoughts. > > > > > > Changes since v1 [2]: > > > > Thanks for all this work Aaron! Can you drop the RFC prefix, > > rebase onto linus' latest tree (as he already merged my > > modules-next, so his tree is more up to date), and submit again? > > > > I'll then apply this to my modules-next, and then ask Christophe to > > rebase on top of that. > > > > Michal, you'd be up next if you want to go through modules-next. > > Sounds like a good idea. When rebasing on top of 5.17-rc1 the only > conflict was on the module code. I'll let you know once modules-next is ready for your code. But before that, does anyone have any objections with this code going through modules-next? Although its kexec related it touches on a lot of kernel/module.c and if we don't take it on modules-next I'm afraid there will be quite a bit of conflicts there later. Luis
On Tue 2022-02-01 08:44 -0800, Allen wrote:
> Build/boot test on qemu. Running more tests on BM.
Thanks Allen.
Kind regards,
On Tue 2022-02-01 18:44 -0800, Luis Chamberlain wrote: > Nice, can you get this tested with 0day? You can ask for your tree to > be tested, see: > > https://01.org/lkp/documentation/0-day-test-service > > See the question, "Which git tree and which mailing list will be tested? > How can I opt-in or opt-out from it?" Thanks for this information Luis. Kind regards,
On Wed 2022-02-02 16:20 -0800, Luis Chamberlain wrote: > Thanks for all this work Aaron! Can you drop the RFC prefix, > rebase onto linus' latest tree (as he already merged my > modules-next, so his tree is more up to date), and submit again? No problem and will do. > I'll then apply this to my modules-next, and then ask Christophe to > rebase on top of that. > > Michal, you'd be up next if you want to go through modules-next. > > Aaron, please Cc Christophe and Michal on your next respin. Sure. Kind regards,
On Thu 2022-02-03 07:48 +0000, Christophe Leroy wrote: > All function prototypes in header files are pointlessly prepended with > 'extern' keyword. This was done that way in the old days, but has been > deprecated as this keyword does nothing on function prototypes but adds > visual pollution when looking at the files. Christophe, Firstly, thanks for your initial feedback. I will address the above. Kind regards,
On Thu 2022-02-03 18:01 +0000, Christophe Leroy wrote: > I have another comment: I think patch 5 should be dropped. Fair enough. > Having something behave based on a CONFIG_ARCH_HAS_SOMETHING item is > wrong. It is not because a plateform selects > CONFIG_ARCH_HAS_STRICT_MODULE_RWX that the module core should behave > differentely than with other platforms as far as the user has not > selected CONFIG_STRICT_MODULE_RWX. > > And the topic here is wrong. It is a coincidence if making that stuff > depend on CONFIG_ARCH_HAS_STRICT_MODULE_RWX works. This is just because > the only architectures that do the module allocation without Exec flag > are architectures that have also selected > CONFIG_ARCH_HAS_STRICT_MODULE_RWX. But it should also work on other > architectures. > > I don't know exactly what was the motivation for commit 93651f80dcb6 > ("modules: fix compile error if don't have strict module rwx") at the > first place but it is just wrong and we should fix it. > > module_enable_x() should work just fine regardless of > CONFIG_ARCH_HAS_STRICT_MODULE_RWX. The above does make sense, to me. Kind regards,
On Thu 2022-02-03 18:15 +0000, Christophe Leroy wrote: > I also have the feeling that a lot of stuff like function prototypes you > added in include/linux/module.h should instead go in > kernel/module/internal.h as they are not used outside of kernel/module/ Agreed - this will be reflected in v5. Kind regards,
On Thu 2022-02-03 12:10 -0800, Luis Chamberlain wrote: > Linus now merged the fix in question, just be sure to use his > latest tree, it should include 67d6212afda218d564890d1674bab28e8612170f Hi Luis, Sure, will do. > If you can fix the issues from your patches which Christophe mentioned > that would be great. Then I'll apply then and then Christophe can work > off of that. All right. I will try to complete this shortly. Kind regards,
On Thu 2022-02-03 18:01 +0000, Christophe Leroy wrote: > I don't know exactly what was the motivation for commit 93651f80dcb6 > ("modules: fix compile error if don't have strict module rwx") at the > first place but it is just wrong and we should fix it. Christophe, I think we are in agreement. If I understand correctly, it should not be possible to enable CONFIG_STRICT_MODULE_RWX without CONFIG_ARCH_HAS_STRICT_MODULE_RWX (or inversely), as per arch/Kconfig: config STRICT_MODULE_RWX bool "Set loadable kernel module data as NX and text as RO" if ARCH_OPTIONAL_KERNEL_RWX depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES default !ARCH_OPTIONAL_KERNEL_RWX || ARCH_OPTIONAL_KERNEL_RWX_DEFAULT The objective of Linus' commit ad21fc4faa2a1 ("arch: Move CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common") and in particular commit 0f5bf6d0afe4b ("arch: Rename CONFIG_DEBUG_RODATA and CONFIG_DEBUG_MODULE_RONX") does seem correct. So, architectures that would prefer to make this feature selectable rather than enabled by default should continue to have this option. > module_enable_x() should work just fine regardless of > CONFIG_ARCH_HAS_STRICT_MODULE_RWX. As per the above, we should fix commit 93651f80dcb6 ("modules: fix compile error if don't have strict module rwx") so a stub for module_enable_x() would no longer be required, right? Kind regards,
Le 07/02/2022 à 17:46, Aaron Tomlin a écrit : > On Thu 2022-02-03 18:01 +0000, Christophe Leroy wrote: >> I don't know exactly what was the motivation for commit 93651f80dcb6 >> ("modules: fix compile error if don't have strict module rwx") at the >> first place but it is just wrong and we should fix it. > > Christophe, > > I think we are in agreement. If I understand correctly, it should not be > possible to enable CONFIG_STRICT_MODULE_RWX without > CONFIG_ARCH_HAS_STRICT_MODULE_RWX (or inversely), as per arch/Kconfig: > > config STRICT_MODULE_RWX > bool "Set loadable kernel module data as NX and text as RO" if ARCH_OPTIONAL_KERNEL_RWX > depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES > default !ARCH_OPTIONAL_KERNEL_RWX || ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > > The objective of Linus' commit ad21fc4faa2a1 ("arch: Move > CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common") and in > particular commit 0f5bf6d0afe4b ("arch: Rename CONFIG_DEBUG_RODATA and > CONFIG_DEBUG_MODULE_RONX") does seem correct. So, architectures that would > prefer to make this feature selectable rather than enabled by default > should continue to have this option. > >> module_enable_x() should work just fine regardless of >> CONFIG_ARCH_HAS_STRICT_MODULE_RWX. > > As per the above, we should fix commit 93651f80dcb6 ("modules: fix compile > error if don't have strict module rwx") so a stub for module_enable_x() > would no longer be required, right? > Yes and that's the purpose of the patch I proposed at https://patchwork.kernel.org/project/linux-modules/patch/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu/ Allthough I need to find out what's the problem reported by the robot. As suggested by Luis, this fix should go once all ongoing work is done. But it would be nice if you could just remove patch 5 from you series, otherwise we would have to revert it later. Thanks Christophe
On Mon 2022-02-07 17:17 +0000, Christophe Leroy wrote: > Yes and that's the purpose of the patch I proposed at > https://patchwork.kernel.org/project/linux-modules/patch/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu/ I see. > Allthough I need to find out what's the problem reported by the robot. I'll have a look too. > As suggested by Luis, this fix should go once all ongoing work is done. > But it would be nice if you could just remove patch 5 from you series, > otherwise we would have to revert it later. Perhaps it might be easier if I keep the patch within the series; once merged into module-next, by Luis, you can rebase and then add the "Fixes:" tag to resolve the issue, no? Kind regards,
> -----Message d'origine----- > De : Aaron Tomlin <atomlin@redhat.com> > Envoyé : lundi 7 février 2022 19:02 > À : Christophe Leroy <christophe.leroy@csgroup.eu> > Cc : Aaron Tomlin <atomlin@atomlin.com>; Luis Chamberlain > <mcgrof@kernel.org>; Michal Suchanek <msuchanek@suse.de>; cl@linux.com; > pmladek@suse.com; mbenes@suse.cz; akpm@linux-foundation.org; > jeyu@kernel.org; linux-kernel@vger.kernel.org; linux-modules@vger.kernel.org; > live-patching@vger.kernel.org; ghalat@redhat.com; allen.lkml@gmail.com; > void@manifault.com; joe@perches.com > Objet : Re: [RFC PATCH v4 00/13] module: core code clean up > > On Mon 2022-02-07 17:17 +0000, Christophe Leroy wrote: > > Yes and that's the purpose of the patch I proposed at > > https://patchwork.kernel.org/project/linux- > modules/patch/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.gi > t.christophe.leroy@csgroup.eu/ > > I see. > > > Allthough I need to find out what's the problem reported by the robot. > > I'll have a look too. > > > As suggested by Luis, this fix should go once all ongoing work is done. > > But it would be nice if you could just remove patch 5 from you series, > > otherwise we would have to revert it later. > > Perhaps it might be easier if I keep the patch within the series; once > merged into module-next, by Luis, you can rebase and then add the "Fixes:" > tag to resolve the issue, no? I don't think it is easier. If we do that it means we'll move some code from main.c to arch_strict_rwx.c with your series, then move it back to main.c and remove arch_strict_rwx.c when we do the fix. That's not good for history tracking because the code we move back and forth will appear as new code in main.c whereas it's code that has been there for years. As we know the code will be back in main.c at the end, I looks easier to me to not move it at all by not applying your patch 5. Thanks Christophe
On Tue 2022-02-08 07:50 +0000, Christophe Leroy wrote: > As we know the code will be back in main.c at the end, I looks easier to > me to not move it at all by not applying your patch 5. Looking at your proposed solution once again, I do agree now. Kind regards,
Hi Luis, As per your suggestion [1], this is an attempt to refactor and split optional code out of core module support code into separate components. This version is based on branch mcgrof/modules-next since a97ac8cb24a3/or modules-5.17-rc1. Please let me know your thoughts. Changes since v1 [2]: - Moved module version support code into a new file Changes since v2 [3]: - Moved module decompress support to a separate file - Made check_modinfo_livepatch() generic (Petr Mladek) - Removed filename from each newly created file (Luis Chamberlain) - Addressed some (i.e. --ignore=ASSIGN_IN_IF,AVOID_BUG was used) minor scripts/checkpatch.pl concerns e.g., use strscpy over strlcpy and missing a blank line after declarations (Allen) Changes since v3 [4]: - Refactored both is_livepatch_module() and set_livepatch_module(), respectively, to use IS_ENABLED(CONFIG_LIVEPATCH) (Joe Perches) - Addressed various compiler warnings e.g., no previous prototype (0-day) [1]: https://lore.kernel.org/lkml/YbEZ4HgSYQEPuRmS@bombadil.infradead.org/ [2]: https://lore.kernel.org/lkml/20211228213041.1356334-1-atomlin@redhat.com/ [3]: https://lore.kernel.org/lkml/20220106234319.2067842-1-atomlin@redhat.com/ [4]: https://lore.kernel.org/lkml/20220128203934.600247-1-atomlin@redhat.com/ Aaron Tomlin (13): module: Move all into module/ module: Simple refactor in preparation for split module: Move livepatch support to a separate file module: Move latched RB-tree support to a separate file module: Move arch strict rwx support to a separate file module: Move strict rwx support to a separate file module: Move extra signature support out of core code module: Move kmemleak support to a separate file module: Move kallsyms support into a separate file module: Move procfs support into a separate file module: Move sysfs support into a separate file module: Move kdb_modules list out of core code module: Move version support into a separate file MAINTAINERS | 2 +- include/linux/module.h | 64 +- kernel/Makefile | 5 +- kernel/debug/kdb/kdb_main.c | 5 + kernel/module-internal.h | 50 - kernel/module/Makefile | 20 + kernel/module/arch_strict_rwx.c | 44 + kernel/module/debug_kmemleak.c | 30 + .../decompress.c} | 2 +- kernel/module/internal.h | 236 +++ kernel/module/kallsyms.c | 502 +++++ kernel/module/livepatch.c | 74 + kernel/{module.c => module/main.c} | 1874 +---------------- kernel/module/procfs.c | 142 ++ .../signature.c} | 0 kernel/module/signing.c | 120 ++ kernel/module/strict_rwx.c | 83 + kernel/module/sysfs.c | 425 ++++ kernel/module/tree_lookup.c | 109 + kernel/module/version.c | 110 + kernel/module_signing.c | 45 - 21 files changed, 2038 insertions(+), 1904 deletions(-) delete mode 100644 kernel/module-internal.h create mode 100644 kernel/module/Makefile create mode 100644 kernel/module/arch_strict_rwx.c create mode 100644 kernel/module/debug_kmemleak.c rename kernel/{module_decompress.c => module/decompress.c} (99%) create mode 100644 kernel/module/internal.h create mode 100644 kernel/module/kallsyms.c create mode 100644 kernel/module/livepatch.c rename kernel/{module.c => module/main.c} (63%) create mode 100644 kernel/module/procfs.c rename kernel/{module_signature.c => module/signature.c} (100%) create mode 100644 kernel/module/signing.c create mode 100644 kernel/module/strict_rwx.c create mode 100644 kernel/module/sysfs.c create mode 100644 kernel/module/tree_lookup.c create mode 100644 kernel/module/version.c delete mode 100644 kernel/module_signing.c base-commit: a97ac8cb24a3c3ad74794adb83717ef1605d1b47