linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
@ 2025-10-31  8:09 Coiby Xu
  2025-10-31 20:09 ` Aaron Tomlin
  2025-11-01 22:10 ` Daniel Gomez
  0 siblings, 2 replies; 5+ messages in thread
From: Coiby Xu @ 2025-10-31  8:09 UTC (permalink / raw)
  To: linux-modules
  Cc: linux-integrity, kernel test robot, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, open list:MODULE SUPPORT

Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
is enabled. This can lead to a linking error if
set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,

    ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
    security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'

So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
enabled.

Note this issue hasn't caused a real problem because all current callers
of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
depend on CONFIG_MODULE_SIG=y.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/module.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index e135cc79acee..fa251958b138 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -769,8 +769,6 @@ static inline bool is_livepatch_module(struct module *mod)
 #endif
 }
 
-void set_module_sig_enforced(void);
-
 void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
 
 #else /* !CONFIG_MODULES... */
@@ -865,10 +863,6 @@ static inline bool module_requested_async_probing(struct module *module)
 }
 
 
-static inline void set_module_sig_enforced(void)
-{
-}
-
 /* Dereference module function descriptor */
 static inline
 void *dereference_module_function_descriptor(struct module *mod, void *ptr)
@@ -924,6 +918,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
 #ifdef CONFIG_MODULE_SIG
 bool is_module_sig_enforced(void);
 
+void set_module_sig_enforced(void);
+
 static inline bool module_sig_ok(struct module *module)
 {
 	return module->sig_ok;
@@ -934,6 +930,10 @@ static inline bool is_module_sig_enforced(void)
 	return false;
 }
 
+static inline void set_module_sig_enforced(void)
+{
+}
+
 static inline bool module_sig_ok(struct module *module)
 {
 	return true;

base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
  2025-10-31  8:09 [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y Coiby Xu
@ 2025-10-31 20:09 ` Aaron Tomlin
  2025-11-04  3:05   ` Coiby Xu
  2025-11-01 22:10 ` Daniel Gomez
  1 sibling, 1 reply; 5+ messages in thread
From: Aaron Tomlin @ 2025-10-31 20:09 UTC (permalink / raw)
  To: Coiby Xu
  Cc: linux-modules, linux-integrity, kernel test robot,
	Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	open list:MODULE SUPPORT

On Fri, Oct 31, 2025 at 04:09:48PM +0800, Coiby Xu wrote:
> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
> is enabled. This can lead to a linking error if
> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
> 
>     ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>     security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
> 
> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
> enabled.
> 
> Note this issue hasn't caused a real problem because all current callers
> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
> depend on CONFIG_MODULE_SIG=y.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>  include/linux/module.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e135cc79acee..fa251958b138 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -769,8 +769,6 @@ static inline bool is_livepatch_module(struct module *mod)
>  #endif
>  }
>  
> -void set_module_sig_enforced(void);
> -
>  void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
>  
>  #else /* !CONFIG_MODULES... */
> @@ -865,10 +863,6 @@ static inline bool module_requested_async_probing(struct module *module)
>  }
>  
>  
> -static inline void set_module_sig_enforced(void)
> -{
> -}
> -
>  /* Dereference module function descriptor */
>  static inline
>  void *dereference_module_function_descriptor(struct module *mod, void *ptr)
> @@ -924,6 +918,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
>  #ifdef CONFIG_MODULE_SIG
>  bool is_module_sig_enforced(void);
>  
> +void set_module_sig_enforced(void);
> +
>  static inline bool module_sig_ok(struct module *module)
>  {
>  	return module->sig_ok;
> @@ -934,6 +930,10 @@ static inline bool is_module_sig_enforced(void)
>  	return false;
>  }
>  
> +static inline void set_module_sig_enforced(void)
> +{
> +}
> +
>  static inline bool module_sig_ok(struct module *module)
>  {
>  	return true;
> 
> base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
> -- 
> 2.51.0
> 
> 

Looks good to me.

Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

-- 
Aaron Tomlin

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
  2025-10-31  8:09 [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y Coiby Xu
  2025-10-31 20:09 ` Aaron Tomlin
@ 2025-11-01 22:10 ` Daniel Gomez
  2025-11-04  3:04   ` Coiby Xu
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Gomez @ 2025-11-01 22:10 UTC (permalink / raw)
  To: Coiby Xu, linux-modules
  Cc: linux-integrity, kernel test robot, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, open list:MODULE SUPPORT

On 31/10/2025 09.09, Coiby Xu wrote:
> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
> is enabled. This can lead to a linking error if
> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
> 
>     ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>     security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'

It's a bit unclear whether you're referring to a current upstream issue (which I
couldn't find as of -rc3), or if this is just a hypothetical scenario.

> 
> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
> enabled.

I only see cases where code has a safeguard like in
security/integrity/ima/ima_efi.c:71

		if (IS_ENABLED(CONFIG_MODULE_SIG))
			set_module_sig_enforced();

> 
> Note this issue hasn't caused a real problem because all current callers
> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
> depend on CONFIG_MODULE_SIG=y.

I think the correct term we should use here is runtime safeguard. The code does
not actually depend on that config, nor is there any dep in Kconfig.

> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
> Signed-off-by: Coiby Xu <coxu@redhat.com>


Just minor nits regarding the commit message structure. This change should allow
us to remove the safeguard from users of set_module_sig_enforced().


Other than that, LGTM,

Reviewed-by: Daniel Gomez <da.gomez@samsung.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
  2025-11-01 22:10 ` Daniel Gomez
@ 2025-11-04  3:04   ` Coiby Xu
  0 siblings, 0 replies; 5+ messages in thread
From: Coiby Xu @ 2025-11-04  3:04 UTC (permalink / raw)
  To: Daniel Gomez
  Cc: linux-modules, linux-integrity, kernel test robot,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
	open list:MODULE SUPPORT

On Sat, Nov 01, 2025 at 11:10:51PM +0100, Daniel Gomez wrote:
>On 31/10/2025 09.09, Coiby Xu wrote:
>> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
>> is enabled. This can lead to a linking error if
>> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
>>
>>     ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>>     security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
>
>It's a bit unclear whether you're referring to a current upstream issue (which I
>couldn't find as of -rc3), or if this is just a hypothetical scenario.

Hi Daniel,

Yes, this issue is hypothetical and currently doesn't cause any real
trouble. lkp found this issue in one of my proposed patches
https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
But I'll use a different solution so the above patch will be abandoned
and will not be applied.

>
>>
>> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
>> enabled.
>
>I only see cases where code has a safeguard like in
>security/integrity/ima/ima_efi.c:71
>
>		if (IS_ENABLED(CONFIG_MODULE_SIG))
>			set_module_sig_enforced();
>
>>
>> Note this issue hasn't caused a real problem because all current callers
>> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
>> depend on CONFIG_MODULE_SIG=y.
>
>I think the correct term we should use here is runtime safeguard. The code does
>not actually depend on that config, nor is there any dep in Kconfig.

Thanks for correcting me! Sorry I didn't realize the constant folding
compiler optimization and made a false claim while forgetting the fact
security/integrity/ima/ima_efi.c also explicitly use
"#if !IS_ENABLED(CONFIG_MODULE_SIG)".

>
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>
>Just minor nits regarding the commit message structure. This change should allow
>us to remove the safeguard from users of set_module_sig_enforced().

Thanks for the suggestion! Does the following commit address address you
concern?

     module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
     
     Currently if set_module_sig_enforced is called with CONFIG_MODULE_SIG=n
     e.g. [1], it can lead to a linking error,
     
         ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
         security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
     
     This happens because the actual implementation of
     set_module_sig_enforced comes from CONFIG_MODULE_SIG but both the
     function declaration and the empty stub definition are tied to
     CONFIG_MODULES.
     
     So bind set_module_sig_enforced to CONFIG_MODULE_SIG instead. This
     allows (future) users to call set_module_sig_enforced directly without
     the "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
     
     Note this issue hasn't caused a real problem because all current callers
     of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
     use "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
     
     [1] https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
     
     Reported-by: kernel test robot <lkp@intel.com>
     Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/

>
>
>Other than that, LGTM,
>
>Reviewed-by: Daniel Gomez <da.gomez@samsung.com>

Thanks for reviewing the patch!

>

-- 
Best regards,
Coiby


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
  2025-10-31 20:09 ` Aaron Tomlin
@ 2025-11-04  3:05   ` Coiby Xu
  0 siblings, 0 replies; 5+ messages in thread
From: Coiby Xu @ 2025-11-04  3:05 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: linux-modules, linux-integrity, kernel test robot,
	Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	open list:MODULE SUPPORT

On Fri, Oct 31, 2025 at 04:09:27PM -0400, Aaron Tomlin wrote:
>On Fri, Oct 31, 2025 at 04:09:48PM +0800, Coiby Xu wrote:
>> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
>> is enabled. This can lead to a linking error if
>> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
>>
>>     ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>>     security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
>>
>> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
>> enabled.
>>
>> Note this issue hasn't caused a real problem because all current callers
>> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
>> depend on CONFIG_MODULE_SIG=y.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>> ---
>>  include/linux/module.h | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index e135cc79acee..fa251958b138 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -769,8 +769,6 @@ static inline bool is_livepatch_module(struct module *mod)
>>  #endif
>>  }
>>
>> -void set_module_sig_enforced(void);
>> -
>>  void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
>>
>>  #else /* !CONFIG_MODULES... */
>> @@ -865,10 +863,6 @@ static inline bool module_requested_async_probing(struct module *module)
>>  }
>>
>>
>> -static inline void set_module_sig_enforced(void)
>> -{
>> -}
>> -
>>  /* Dereference module function descriptor */
>>  static inline
>>  void *dereference_module_function_descriptor(struct module *mod, void *ptr)
>> @@ -924,6 +918,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
>>  #ifdef CONFIG_MODULE_SIG
>>  bool is_module_sig_enforced(void);
>>
>> +void set_module_sig_enforced(void);
>> +
>>  static inline bool module_sig_ok(struct module *module)
>>  {
>>  	return module->sig_ok;
>> @@ -934,6 +930,10 @@ static inline bool is_module_sig_enforced(void)
>>  	return false;
>>  }
>>
>> +static inline void set_module_sig_enforced(void)
>> +{
>> +}
>> +
>>  static inline bool module_sig_ok(struct module *module)
>>  {
>>  	return true;
>>
>> base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
>> --
>> 2.51.0
>>
>>
>
>Looks good to me.
>
>Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

Hi Aaron,

Thanks for reviewing the patch!

>
>-- 
>Aaron Tomlin
>

-- 
Best regards,
Coiby


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-04  3:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-31  8:09 [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y Coiby Xu
2025-10-31 20:09 ` Aaron Tomlin
2025-11-04  3:05   ` Coiby Xu
2025-11-01 22:10 ` Daniel Gomez
2025-11-04  3:04   ` Coiby Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).