Re: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

2023-10-12 Thread kernel test robot
Hi Joey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mcgrof/modules-next]
[also build test WARNING on linus/master v6.6-rc5 next-20231012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Joey-Jiao/module-Add-CONFIG_MODULE_LOAD_IN_SEQUENCE-option/20231011-154640
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git 
modules-next
patch link:
https://lore.kernel.org/r/20231011074438.6098-1-quic_jiangenj%40quicinc.com
patch subject: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option
config: um-i386_defconfig 
(https://download.01.org/0day-ci/archive/20231013/202310130206.f778hunp-...@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231013/202310130206.f778hunp-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202310130206.f778hunp-...@intel.com/

All warnings (new ones prefixed by >>):

   kernel/module/main.c: In function 'do_init_module':
   kernel/module/main.c:2627:12: error: invalid storage class for function 
'may_init_module'
static int may_init_module(void)
   ^~~
   kernel/module/main.c:2636:13: error: invalid storage class for function 
'finished_loading'
static bool finished_loading(const char *name)
^~~~
   kernel/module/main.c:2657:12: error: invalid storage class for function 
'module_patient_check_exists'
static int module_patient_check_exists(const char *name,
   ^~~
   kernel/module/main.c:2701:12: error: invalid storage class for function 
'add_unformed_module'
static int add_unformed_module(struct module *mod)
   ^~~
   kernel/module/main.c:2722:12: error: invalid storage class for function 
'complete_formation'
static int complete_formation(struct module *mod, struct load_info *info)
   ^~
   kernel/module/main.c:2755:12: error: invalid storage class for function 
'prepare_coming_module'
static int prepare_coming_module(struct module *mod)
   ^
   kernel/module/main.c:2773:12: error: invalid storage class for function 
'unknown_module_param_cb'
static int unknown_module_param_cb(char *param, char *val, const char 
*modname,
   ^~~
   kernel/module/main.c:2793:12: error: invalid storage class for function 
'early_mod_check'
static int early_mod_check(struct load_info *info, int flags)
   ^~~
   kernel/module/main.c:2829:12: error: invalid storage class for function 
'load_module'
static int load_module(struct load_info *info, const char __user *uargs,
   ^~~
>> kernel/module/main.c:3039:1: warning: 'alias' attribute ignored 
>> [-Wattributes]
SYSCALL_DEFINE3(init_module, void __user *, umod,
^~~
   In file included from kernel/module/main.c:26:0:
   include/linux/syscalls.h:247:21: error: invalid storage class for function 
'__do_sys_init_module'
 static inline long __do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
^
   include/linux/syscalls.h:230:2: note: in expansion of macro 
'__SYSCALL_DEFINEx'
 __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
 ^
   include/linux/syscalls.h:221:36: note: in expansion of macro 
'SYSCALL_DEFINEx'
#define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
   ^~~
   kernel/module/main.c:3039:1: note: in expansion of macro 'SYSCALL_DEFINE3'
SYSCALL_DEFINE3(init_module, void __user *, umod,
^~~
   include/linux/syscalls.h:249:18: error: static declaration of 
'__se_sys_init_module' follows non-static declaration
 asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
 ^
   include/linux/syscalls.h:230:2: note: in expansion of macro 
'__SYSCALL_DEFINEx'
 __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
 ^
   include/linux/syscalls.h:221:36: note: in expansion of macro 
'SYSCALL_DEFINEx'
#define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
   ^~~
   kernel/module/main.c:3039:1: note: in expansion of macro 'SYSCALL_DEFINE3'
SYSCALL_DEFINE3(init_module, void __user *, umod,
^~~
   include/linux/syscalls.h:248:18: note: previous declaration of 
'__se_sys_init_module' was here
 asmlinkage long __se

Re: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

2023-10-12 Thread kernel test robot
Hi Joey,

kernel test robot noticed the following build errors:

[auto build test ERROR on mcgrof/modules-next]
[also build test ERROR on linus/master v6.6-rc5 next-20231012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Joey-Jiao/module-Add-CONFIG_MODULE_LOAD_IN_SEQUENCE-option/20231011-154640
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git 
modules-next
patch link:
https://lore.kernel.org/r/20231011074438.6098-1-quic_jiangenj%40quicinc.com
patch subject: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option
config: um-allnoconfig 
(https://download.01.org/0day-ci/archive/20231013/202310130236.lybpy0lh-...@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 
4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231013/202310130236.lybpy0lh-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202310130236.lybpy0lh-...@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/module/main.c:14:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 547 | val = __raw_readb(PCI_IOBASE + addr);
 |   ~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + 
addr));
 | ~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from 
macro '__le16_to_cpu'
  37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
 |   ^
   In file included from kernel/module/main.c:14:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + 
addr));
 | ~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from 
macro '__le32_to_cpu'
  35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
 |   ^
   In file included from kernel/module/main.c:14:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 584 | __raw_writeb(value, PCI_IOBASE + addr);
 | ~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + 
addr);
 |   ~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a 
null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + 
addr);
 |   ~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arit

Re: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

2023-10-11 Thread Luis Chamberlain
On Wed, Oct 11, 2023 at 01:14:38PM +0530, Joey Jiao wrote:
> When modprobe cmds are executed one by one, the final loaded modules
> are not in fixed sequence as expected.
> 
> Add the option to make sure modules are in fixed sequence across reboot.
> 
> Signed-off-by: Joey Jiao 
> ---
>  kernel/module/Kconfig | 11 +++
>  kernel/module/main.c  |  3 ++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 33a2e991f608..b45a45f31d6d 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -389,4 +389,15 @@ config MODULES_TREE_LOOKUP
>   def_bool y
>   depends on PERF_EVENTS || TRACING || CFI_CLANG
>  
> +config MODULE_LOAD_IN_SEQUENCE
> + bool "Load module in sequence"
> + default n
> + help
> +   By default, modules are loaded in random sequence depending on when 
> modprobe
> +   is executed.
> +
> +   This option allows modules to be loaded in sequence if modprobe cmds 
> are
> +   executed one by one in sequence. This option is helpful during 
> syzkaller fuzzing
> +   to make sure module is loaded into fixed address across device reboot.
> +
>  endif # MODULES
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 98fedfdb8db5..e238a31d09eb 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2594,7 +2594,8 @@ static noinline int do_init_module(struct module *mod)
>* rcu_barrier()
>*/
>   if (llist_add(>node, _free_list))
> - schedule_work(_free_wq);
> + if (!IS_ENABLED(CONFIG_MODULE_LOAD_IN_SEQUENCE)) {
> + schedule_work(_free_wq);
>  

As Christoph suggested the rationale for something like this needs to be
clearly spelled out in the commit log and if so valuable it should be
a default. The commit log and even the Kconfig description do little
to justify any rationale for this.

  Luis



Re: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

2023-10-11 Thread kernel test robot
Hi Joey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mcgrof/modules-next]
[also build test WARNING on linus/master v6.6-rc5 next-20231011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Joey-Jiao/module-Add-CONFIG_MODULE_LOAD_IN_SEQUENCE-option/20231011-154640
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git 
modules-next
patch link:
https://lore.kernel.org/r/20231011074438.6098-1-quic_jiangenj%40quicinc.com
patch subject: [PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option
config: m68k-allyesconfig 
(https://download.01.org/0day-ci/archive/20231011/202310111840.ufgoxyfm-...@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231011/202310111840.ufgoxyfm-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202310111840.ufgoxyfm-...@intel.com/

All warnings (new ones prefixed by >>):

   kernel/module/main.c: In function 'do_init_module':
   kernel/module/main.c:2627:12: error: invalid storage class for function 
'may_init_module'
2627 | static int may_init_module(void)
 |^~~
   kernel/module/main.c:2636:13: error: invalid storage class for function 
'finished_loading'
2636 | static bool finished_loading(const char *name)
 | ^~~~
   kernel/module/main.c:2657:12: error: invalid storage class for function 
'module_patient_check_exists'
2657 | static int module_patient_check_exists(const char *name,
 |^~~
   kernel/module/main.c:2701:12: error: invalid storage class for function 
'add_unformed_module'
2701 | static int add_unformed_module(struct module *mod)
 |^~~
   kernel/module/main.c:2722:12: error: invalid storage class for function 
'complete_formation'
2722 | static int complete_formation(struct module *mod, struct load_info 
*info)
 |^~
   kernel/module/main.c:2755:12: error: invalid storage class for function 
'prepare_coming_module'
2755 | static int prepare_coming_module(struct module *mod)
 |^
   kernel/module/main.c:2773:12: error: invalid storage class for function 
'unknown_module_param_cb'
2773 | static int unknown_module_param_cb(char *param, char *val, const 
char *modname,
 |^~~
   kernel/module/main.c:2793:12: error: invalid storage class for function 
'early_mod_check'
2793 | static int early_mod_check(struct load_info *info, int flags)
 |^~~
   kernel/module/main.c:2829:12: error: invalid storage class for function 
'load_module'
2829 | static int load_module(struct load_info *info, const char __user 
*uargs,
 |^~~
   In file included from include/linux/compiler_types.h:125,
from :
>> include/linux/compiler-gcc.h:132:33: warning: 'alias' attribute ignored 
>> [-Wattributes]
 132 | #define __diag(s)   _Pragma(__diag_str(GCC diagnostic s))
 | ^~~
   include/linux/compiler-gcc.h:135:33: note: in expansion of macro '__diag'
 135 | #define __diag_GCC_8(s) __diag(s)
 | ^~
   include/linux/compiler-gcc.h:123:9: note: in expansion of macro 
'__diag_GCC_8'
 123 | __diag_GCC_ ## version(__diag_GCC_ ## severity s)
 | ^~~
   include/linux/compiler_types.h:416:9: note: in expansion of macro 
'__diag_GCC'
 416 | __diag_ ## compiler(version, ignore, option)
 | ^~~
   include/linux/syscalls.h:242:9: note: in expansion of macro '__diag_ignore'
 242 | __diag_ignore(GCC, 8, "-Wattribute-alias",   
   \
 | ^
   include/linux/syscalls.h:230:9: note: in expansion of macro 
'__SYSCALL_DEFINEx'
 230 | __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
 | ^
   include/linux/syscalls.h:221:36: note: in expansion of macro 
'SYSCALL_DEFINEx'
 221 | #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, 
__VA_ARGS__)
 |^~~
   kernel/module/main.c:3039:1: note: in expansion of macro 'SYSCALL_DEFINE3'
3039 | SYSCALL_DEFINE3(init_module, void __user *, umod,
 | ^~~
   In file included from kernel/module

[PATCH v2] module: Add CONFIG_MODULE_LOAD_IN_SEQUENCE option

2023-10-11 Thread Joey Jiao
When modprobe cmds are executed one by one, the final loaded modules
are not in fixed sequence as expected.

Add the option to make sure modules are in fixed sequence across reboot.

Signed-off-by: Joey Jiao 
---
 kernel/module/Kconfig | 11 +++
 kernel/module/main.c  |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 33a2e991f608..b45a45f31d6d 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -389,4 +389,15 @@ config MODULES_TREE_LOOKUP
def_bool y
depends on PERF_EVENTS || TRACING || CFI_CLANG
 
+config MODULE_LOAD_IN_SEQUENCE
+   bool "Load module in sequence"
+   default n
+   help
+ By default, modules are loaded in random sequence depending on when 
modprobe
+ is executed.
+
+ This option allows modules to be loaded in sequence if modprobe cmds 
are
+ executed one by one in sequence. This option is helpful during 
syzkaller fuzzing
+ to make sure module is loaded into fixed address across device reboot.
+
 endif # MODULES
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..e238a31d09eb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2594,7 +2594,8 @@ static noinline int do_init_module(struct module *mod)
 * rcu_barrier()
 */
if (llist_add(>node, _free_list))
-   schedule_work(_free_wq);
+   if (!IS_ENABLED(CONFIG_MODULE_LOAD_IN_SEQUENCE)) {
+   schedule_work(_free_wq);
 
mutex_unlock(_mutex);
wake_up_all(_wq);
-- 
2.42.0