Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Baoquan He]

CC kexec list.

On 05/16/25 at 07:39am, Baoquan He wrote:
> Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> extra memory. It would be very helpful to allow IMA to be disabled for
> kdump kernel.
> 
> And Coiby also mentioned that for kdump kernel incorrect ima-policy loaded
> by systemd could cause kdump kernel hang, and it's possible the booting
> process may be stopped by a strict, albeit syntax-correct policy and users
> can't log into the system to fix the policy. In these cases, allowing to
> disable IMA is very helpful too for kdump kernel.
> 
> Hence add a knob ima=on|off here to allow people to disable IMA in kdump
> kenrel if needed.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  5 +++++
>  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index d9fd26b95b34..762fb6ddcc24 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2202,6 +2202,11 @@
>  			different crypto accelerators. This option can be used
>  			to achieve best performance for particular HW.
>  
> +	ima=		[IMA] Enable or disable IMA
> +			Format: { "off" | "on" }
> +			Default: "on"
> +			Note that this is only useful for kdump kernel.
> +
>  	init=		[KNL]
>  			Format: <full_path>
>  			Run specified binary instead of /sbin/init as init
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index f3e7ac513db3..07af5c6af138 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -27,6 +27,7 @@
>  #include <linux/fs.h>
>  #include <linux/iversion.h>
>  #include <linux/evm.h>
> +#include <linux/crash_dump.h>
>  
>  #include "ima.h"
>  
> @@ -38,11 +39,27 @@ int ima_appraise;
>  
>  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
>  static int hash_setup_done;
> +static int ima_disabled;
>  
>  static struct notifier_block ima_lsm_policy_notifier = {
>  	.notifier_call = ima_lsm_policy_change,
>  };
>  
> +static int __init ima_setup(char *str)
> +{
> +	if (strncmp(str, "off", 3) == 0)
> +		ima_disabled = 1;
> +	else if (strncmp(str, "on", 2) == 0)
> +		ima_disabled = 0;
> +	else
> +		pr_err("Invalid ima setup option: \"%s\" , please specify ima=on|off.", str);
> +
> +	return 1;
> +}
> +__setup("ima=", ima_setup);
> +
> +
> +
>  static int __init hash_setup(char *str)
>  {
>  	struct ima_template_desc *template_desc = ima_template_desc_current();
> @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
>  {
>  	int error;
>  
> +	if (ima_disabled && is_kdump_kernel()) {
> +		pr_info("IMA functionality is disabled");
> +		return 0;
> +	}
> +
>  	ima_appraise_parse_cmdline();
>  	ima_init_template_list();
>  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> -- 
> 2.41.0
> 

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> CC kexec list.
> 
> On 05/16/25 at 07:39am, Baoquan He wrote:
> > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > extra memory. It would be very helpful to allow IMA to be disabled for
> > kdump kernel.

The real question is not whether kdump needs "IMA", but whether not enabling
IMA in the kdump kernel could be abused.  The comments below don't address
that question but limit/emphasize, as much as possible, turning IMA off is
limited to the kdump kernel.

> > 
> > And Coiby also mentioned that for kdump kernel incorrect ima-policy loaded
> > by systemd could cause kdump kernel hang, and it's possible the booting
> > process may be stopped by a strict, albeit syntax-correct policy and users
> > can't log into the system to fix the policy. In these cases, allowing to
> > disable IMA is very helpful too for kdump kernel.
> > 
> > Hence add a knob ima=on|off here to allow people to disable IMA in kdump
> > kenrel if needed.

^kernel

> > 
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> >  .../admin-guide/kernel-parameters.txt         |  5 +++++
> >  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index d9fd26b95b34..762fb6ddcc24 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2202,6 +2202,11 @@
> >  			different crypto accelerators. This option can be
> > used
> >  			to achieve best performance for particular HW.
> >  
> > +	ima=		[IMA] Enable or disable IMA
> > +			Format: { "off" | "on" }
> > +			Default: "on"
> > +			Note that this is only useful for kdump kernel.

Instead of "useful" I would prefer something clearer like "limited".

> > +
> >  	init=		[KNL]
> >  			Format: <full_path>
> >  			Run specified binary instead of /sbin/init as
> > init
> > diff --git a/security/integrity/ima/ima_main.c
> > b/security/integrity/ima/ima_main.c
> > index f3e7ac513db3..07af5c6af138 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/fs.h>
> >  #include <linux/iversion.h>
> >  #include <linux/evm.h>
> > +#include <linux/crash_dump.h>
> >  
> >  #include "ima.h"
> >  
> > @@ -38,11 +39,27 @@ int ima_appraise;
> >  
> >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> >  static int hash_setup_done;
> > +static int ima_disabled;

Like the ima_hash_algo variable definition above, ima_disabled should be
defined as __ro_after_init.

> >  
> >  static struct notifier_block ima_lsm_policy_notifier = {
> >  	.notifier_call = ima_lsm_policy_change,
> >  };
> >  
> > +static int __init ima_setup(char *str)
> > +{

is_kdump_kernel() should also be called here, before the tests below. 
Something like:

+       if (!is_kdump_kernel()) {
+               pr_info("Warning ima setup option only permitted in kdump");
+               return 1;
+       }

> > +	if (strncmp(str, "off", 3) == 0)
> > +		ima_disabled = 1;
> > +	else if (strncmp(str, "on", 2) == 0)
> > +		ima_disabled = 0;
> > +	else
> > +		pr_err("Invalid ima setup option: \"%s\" , please specify
> > ima=on|off.", str);
> > +
> > +	return 1;
> > +}
> > +__setup("ima=", ima_setup);
> > +
> > +
> > +

Remove the extraneous blank line.

> >  static int __init hash_setup(char *str)
> >  {
> >  	struct ima_template_desc *template_desc =
> > ima_template_desc_current();
> > @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
> >  {
> >  	int error;
> >  
> > +	if (ima_disabled && is_kdump_kernel()) {
> > +		pr_info("IMA functionality is disabled");
> > +		return 0;
> > +	}
> > +

Even with the additional call to is_kdump_kernel() in ima_setup, please keep
the is_kdump_kernel() test here as well.  Even though the code is self
describing, please add a one line comment emphasizing disabling IMA is limited
to kdump.

> >  	ima_appraise_parse_cmdline();
> >  	ima_init_template_list();
> >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > -- 
> > 2.41.0
> > 
> 
> 

thanks,

Mimi

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

In addition, please update the Subject line to be less generic.

thanks,

Mimi


On Wed, 2025-05-21 at 08:54 -0400, Mimi Zohar wrote:
> On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > CC kexec list.
> > 
> > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > > extra memory. It would be very helpful to allow IMA to be disabled for
> > > kdump kernel.
> 
> The real question is not whether kdump needs "IMA", but whether not enabling
> IMA in the kdump kernel could be abused.  The comments below don't address
> that question but limit/emphasize, as much as possible, turning IMA off is
> limited to the kdump kernel.
> 
> > > 
> > > And Coiby also mentioned that for kdump kernel incorrect ima-policy
> > > loaded
> > > by systemd could cause kdump kernel hang, and it's possible the booting
> > > process may be stopped by a strict, albeit syntax-correct policy and
> > > users
> > > can't log into the system to fix the policy. In these cases, allowing to
> > > disable IMA is very helpful too for kdump kernel.
> > > 
> > > Hence add a knob ima=on|off here to allow people to disable IMA in kdump
> > > kenrel if needed.
> 
> ^kernel
> 
> > > 
> > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > ---
> > >  .../admin-guide/kernel-parameters.txt         |  5 +++++
> > >  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > > b/Documentation/admin-guide/kernel-parameters.txt
> > > index d9fd26b95b34..762fb6ddcc24 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -2202,6 +2202,11 @@
> > >  			different crypto accelerators. This option can
> > > be
> > > used
> > >  			to achieve best performance for particular HW.
> > >  
> > > +	ima=		[IMA] Enable or disable IMA
> > > +			Format: { "off" | "on" }
> > > +			Default: "on"
> > > +			Note that this is only useful for kdump kernel.
> 
> Instead of "useful" I would prefer something clearer like "limited".
> 
> > > +
> > >  	init=		[KNL]
> > >  			Format: <full_path>
> > >  			Run specified binary instead of /sbin/init as
> > > init
> > > diff --git a/security/integrity/ima/ima_main.c
> > > b/security/integrity/ima/ima_main.c
> > > index f3e7ac513db3..07af5c6af138 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -27,6 +27,7 @@
> > >  #include <linux/fs.h>
> > >  #include <linux/iversion.h>
> > >  #include <linux/evm.h>
> > > +#include <linux/crash_dump.h>
> > >  
> > >  #include "ima.h"
> > >  
> > > @@ -38,11 +39,27 @@ int ima_appraise;
> > >  
> > >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> > >  static int hash_setup_done;
> > > +static int ima_disabled;
> 
> Like the ima_hash_algo variable definition above, ima_disabled should be
> defined as __ro_after_init.
> 
> > >  
> > >  static struct notifier_block ima_lsm_policy_notifier = {
> > >  	.notifier_call = ima_lsm_policy_change,
> > >  };
> > >  
> > > +static int __init ima_setup(char *str)
> > > +{
> 
> is_kdump_kernel() should also be called here, before the tests below. 
> Something like:
> 
> +       if (!is_kdump_kernel()) {
> +               pr_info("Warning ima setup option only permitted in kdump");
> +               return 1;
> +       }
> 
> > > +	if (strncmp(str, "off", 3) == 0)
> > > +		ima_disabled = 1;
> > > +	else if (strncmp(str, "on", 2) == 0)
> > > +		ima_disabled = 0;
> > > +	else
> > > +		pr_err("Invalid ima setup option: \"%s\" , please
> > > specify
> > > ima=on|off.", str);
> > > +
> > > +	return 1;
> > > +}
> > > +__setup("ima=", ima_setup);
> > > +
> > > +
> > > +
> 
> Remove the extraneous blank line.
> 
> > >  static int __init hash_setup(char *str)
> > >  {
> > >  	struct ima_template_desc *template_desc =
> > > ima_template_desc_current();
> > > @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
> > >  {
> > >  	int error;
> > >  
> > > +	if (ima_disabled && is_kdump_kernel()) {
> > > +		pr_info("IMA functionality is disabled");
> > > +		return 0;
> > > +	}
> > > +
> 
> Even with the additional call to is_kdump_kernel() in ima_setup, please keep
> the is_kdump_kernel() test here as well.  Even though the code is self
> describing, please add a one line comment emphasizing disabling IMA is
> limited
> to kdump.
> 
> > >  	ima_appraise_parse_cmdline();
> > >  	ima_init_template_list();
> > >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > -- 
> > > 2.41.0
> > > 
> > 
> > 
> 
> thanks,
> 
> Mimi
> 

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Baoquan He]

On 05/21/25 at 08:58am, Mimi Zohar wrote:
> In addition, please update the Subject line to be less generic.

Sure, will change subject as below:

ima: add a knob ima= to allow disabling IMA in kdump kernel

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Coiby Xu]

On Wed, May 21, 2025 at 08:54:10AM -0400, Mimi Zohar wrote:
>On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
>> CC kexec list.
>>
>> On 05/16/25 at 07:39am, Baoquan He wrote:
>> > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
>> > extra memory. It would be very helpful to allow IMA to be disabled for
>> > kdump kernel.
>
>The real question is not whether kdump needs "IMA", but whether not enabling
>IMA in the kdump kernel could be abused.  The comments below don't address
>that question but limit/emphasize, as much as possible, turning IMA off is
>limited to the kdump kernel.
>
>> >
>> > And Coiby also mentioned that for kdump kernel incorrect ima-policy loaded
>> > by systemd could cause kdump kernel hang, and it's possible the booting
>> > process may be stopped by a strict, albeit syntax-correct policy and users
>> > can't log into the system to fix the policy. In these cases, allowing to
>> > disable IMA is very helpful too for kdump kernel.

To clarify, what I mentioned early is that the system hangs because
systemd freezes after trying to load an incorrect policy or the booting
process may be stopped by a strict, albeit syntax-correct policy. kdump
won't be affected in these cases because the IMA policy file
(/etc/ima/ima-policy) won't be installed into the kdump initramfs by
default so there is no chance for this IMA policy file to affect kdump.
Besides, if the normal/1st system does hang because of the IMA policy,
the kdump kernel and initramfs simply won't be loaded thus no chance to
to be booted or to load an IMA policy file.

But kdump can be affected if the kernel cmdline parameter
ima_policy=appraise_tcb is configured. Because currently files in kdump
initramfs don't have security.ima. Without the reference value stored in
security.ima to prove a file's integrity, IMA will prevent accessing
this file. So in this case, IMA can also stop systemd from running.

>> >
>> > Hence add a knob ima=on|off here to allow people to disable IMA in kdump
>> > kenrel if needed.
>
>^kernel
>
>> >
>> > Signed-off-by: Baoquan He <bhe@redhat.com>
>> > ---
>> >  .../admin-guide/kernel-parameters.txt         |  5 +++++
>> >  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
>> >  2 files changed, 27 insertions(+)
>> >
>> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
>> > b/Documentation/admin-guide/kernel-parameters.txt
>> > index d9fd26b95b34..762fb6ddcc24 100644
>> > --- a/Documentation/admin-guide/kernel-parameters.txt
>> > +++ b/Documentation/admin-guide/kernel-parameters.txt
>> > @@ -2202,6 +2202,11 @@
>> >  			different crypto accelerators. This option can be
>> > used
>> >  			to achieve best performance for particular HW.
>> >  
>> > +	ima=		[IMA] Enable or disable IMA
>> > +			Format: { "off" | "on" }
>> > +			Default: "on"
>> > +			Note that this is only useful for kdump kernel.
>
>Instead of "useful" I would prefer something clearer like "limited".
>
>> > +
>> >  	init=		[KNL]
>> >  			Format: <full_path>
>> >  			Run specified binary instead of /sbin/init as
>> > init
>> > diff --git a/security/integrity/ima/ima_main.c
>> > b/security/integrity/ima/ima_main.c
>> > index f3e7ac513db3..07af5c6af138 100644
>> > --- a/security/integrity/ima/ima_main.c
>> > +++ b/security/integrity/ima/ima_main.c
>> > @@ -27,6 +27,7 @@
>> >  #include <linux/fs.h>
>> >  #include <linux/iversion.h>
>> >  #include <linux/evm.h>
>> > +#include <linux/crash_dump.h>
>> >  
>> >  #include "ima.h"
>> >  
>> > @@ -38,11 +39,27 @@ int ima_appraise;
>> >  
>> >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
>> >  static int hash_setup_done;
>> > +static int ima_disabled;
>
>Like the ima_hash_algo variable definition above, ima_disabled should be
>defined as __ro_after_init.
>
>> >  
>> >  static struct notifier_block ima_lsm_policy_notifier = {
>> >  	.notifier_call = ima_lsm_policy_change,
>> >  };
>> >  
>> > +static int __init ima_setup(char *str)
>> > +{
>
>is_kdump_kernel() should also be called here, before the tests below.
>Something like:
>
>+       if (!is_kdump_kernel()) {
>+               pr_info("Warning ima setup option only permitted in kdump");
>+               return 1;
>+       }

Yes, this kind of info will be helpful to avoid users misusing
ima=off. I already saw a case where a user tried to use ima=0 to skip
loading an invalid IMA policy file in order to resolve booting failure
"systemd Freezing execution" and even filed a bug saying ima=0 doesn't
work.

>
>> > +	if (strncmp(str, "off", 3) == 0)
>> > +		ima_disabled = 1;
>> > +	else if (strncmp(str, "on", 2) == 0)
>> > +		ima_disabled = 0;
>> > +	else
>> > +		pr_err("Invalid ima setup option: \"%s\" , please specify
>> > ima=on|off.", str);
>> > +
>> > +	return 1;
>> > +}
>> > +__setup("ima=", ima_setup);
>> > +
>> > +
>> > +
>
>Remove the extraneous blank line.
>
>> >  static int __init hash_setup(char *str)
>> >  {
>> >  	struct ima_template_desc *template_desc =
>> > ima_template_desc_current();
>> > @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
>> >  {
>> >  	int error;
>> >  
>> > +	if (ima_disabled && is_kdump_kernel()) {
>> > +		pr_info("IMA functionality is disabled");
>> > +		return 0;
>> > +	}
>> > +
>
>Even with the additional call to is_kdump_kernel() in ima_setup, please keep
>the is_kdump_kernel() test here as well.  Even though the code is self
>describing, please add a one line comment emphasizing disabling IMA is limited
>to kdump.
>
>> >  	ima_appraise_parse_cmdline();
>> >  	ima_init_template_list();
>> >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
>> > --
>> > 2.41.0
>> >
>>
>>
>
>thanks,
>
>Mimi


-- 
Best regards,
Coiby

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Baoquan He]

On 05/21/25 at 08:54am, Mimi Zohar wrote:
> On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > CC kexec list.
> > 
> > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > > extra memory. It would be very helpful to allow IMA to be disabled for
> > > kdump kernel.

Thanks a lot for careufl reviewing and great suggestions.

> 
> The real question is not whether kdump needs "IMA", but whether not enabling
> IMA in the kdump kernel could be abused.  The comments below don't address
> that question but limit/emphasize, as much as possible, turning IMA off is
> limited to the kdump kernel.

Are you suggesting removing below paragraph from patch log because they
are redundant? I can remove it in v2 if yes.

> 
> > > 
> > > And Coiby also mentioned that for kdump kernel incorrect ima-policy loaded
> > > by systemd could cause kdump kernel hang, and it's possible the booting
> > > process may be stopped by a strict, albeit syntax-correct policy and users
> > > can't log into the system to fix the policy. In these cases, allowing to
> > > disable IMA is very helpful too for kdump kernel.
> > > 
> > > Hence add a knob ima=on|off here to allow people to disable IMA in kdump
> > > kenrel if needed.
> 
> ^kernel

Will change.

> 
> > > 
> > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > ---
> > >  .../admin-guide/kernel-parameters.txt         |  5 +++++
> > >  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > > b/Documentation/admin-guide/kernel-parameters.txt
> > > index d9fd26b95b34..762fb6ddcc24 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -2202,6 +2202,11 @@
> > >  			different crypto accelerators. This option can be
> > > used
> > >  			to achieve best performance for particular HW.
> > >  
> > > +	ima=		[IMA] Enable or disable IMA
> > > +			Format: { "off" | "on" }
> > > +			Default: "on"
> > > +			Note that this is only useful for kdump kernel.
> 
> Instead of "useful" I would prefer something clearer like "limited".

Makes sense, will change.

> 
> > > +
> > >  	init=		[KNL]
> > >  			Format: <full_path>
> > >  			Run specified binary instead of /sbin/init as
> > > init
> > > diff --git a/security/integrity/ima/ima_main.c
> > > b/security/integrity/ima/ima_main.c
> > > index f3e7ac513db3..07af5c6af138 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -27,6 +27,7 @@
> > >  #include <linux/fs.h>
> > >  #include <linux/iversion.h>
> > >  #include <linux/evm.h>
> > > +#include <linux/crash_dump.h>
> > >  
> > >  #include "ima.h"
> > >  
> > > @@ -38,11 +39,27 @@ int ima_appraise;
> > >  
> > >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> > >  static int hash_setup_done;
> > > +static int ima_disabled;
> 
> Like the ima_hash_algo variable definition above, ima_disabled should be
> defined as __ro_after_init.

Will add __ro_after_init.

> 
> > >  
> > >  static struct notifier_block ima_lsm_policy_notifier = {
> > >  	.notifier_call = ima_lsm_policy_change,
> > >  };
> > >  
> > > +static int __init ima_setup(char *str)
> > > +{
> 
> is_kdump_kernel() should also be called here, before the tests below. 
> Something like:
> 
> +       if (!is_kdump_kernel()) {
> +               pr_info("Warning ima setup option only permitted in kdump");
> +               return 1;
> +       }

Sure, will change as suggested.

> 
> > > +	if (strncmp(str, "off", 3) == 0)
> > > +		ima_disabled = 1;
> > > +	else if (strncmp(str, "on", 2) == 0)
> > > +		ima_disabled = 0;
> > > +	else
> > > +		pr_err("Invalid ima setup option: \"%s\" , please specify
> > > ima=on|off.", str);
> > > +
> > > +	return 1;
> > > +}
> > > +__setup("ima=", ima_setup);
> > > +
> > > +
> > > +
> 
> Remove the extraneous blank line.

sure.

> 
> > >  static int __init hash_setup(char *str)
> > >  {
> > >  	struct ima_template_desc *template_desc =
> > > ima_template_desc_current();
> > > @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
> > >  {
> > >  	int error;
> > >  
> > > +	if (ima_disabled && is_kdump_kernel()) {
> > > +		pr_info("IMA functionality is disabled");
> > > +		return 0;
> > > +	}
> > > +
> 
> Even with the additional call to is_kdump_kernel() in ima_setup, please keep
> the is_kdump_kernel() test here as well.  Even though the code is self
> describing, please add a one line comment emphasizing disabling IMA is limited
> to kdump.

OK, will keep code here as this v1 is and add one line of comment at
above.

Thanks again.

> 
> > >  	ima_appraise_parse_cmdline();
> > >  	ima_init_template_list();
> > >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > -- 
> > > 2.41.0
> > > 
> > 
> > 
> 
> thanks,
> 
> Mimi
> 

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Coiby Xu]

On Thu, May 22, 2025 at 11:24:13AM +0800, Baoquan He wrote:
>On 05/21/25 at 08:54am, Mimi Zohar wrote:
>> On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
>> > CC kexec list.
>> >
>> > On 05/16/25 at 07:39am, Baoquan He wrote:
>> > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
>> > > extra memory. It would be very helpful to allow IMA to be disabled for
>> > > kdump kernel.
>
>Thanks a lot for careufl reviewing and great suggestions.
>
>>
>> The real question is not whether kdump needs "IMA", but whether not enabling
>> IMA in the kdump kernel could be abused.  The comments below don't address
>> that question but limit/emphasize, as much as possible, turning IMA off is
>> limited to the kdump kernel.
>
>Are you suggesting removing below paragraph from patch log because they
>are redundant? I can remove it in v2 if yes.

I understand Mimi's suggestion as the commit message should answer the
question why disabling IMA should be limited to kdump.

-- 
Best regards,
Coiby

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
> On 05/21/25 at 08:54am, Mimi Zohar wrote:
> > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > > CC kexec list.
> > > 
> > > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > > > extra memory. It would be very helpful to allow IMA to be disabled for
> > > > kdump kernel.
> 
> Thanks a lot for careufl reviewing and great suggestions.
> 
> > 
> > The real question is not whether kdump needs "IMA", but whether not enabling
> > IMA in the kdump kernel could be abused.  The comments below don't address
> > that question but limit/emphasize, as much as possible, turning IMA off is
> > limited to the kdump kernel.
> 
> Are you suggesting removing below paragraph from patch log because they
> are redundant? I can remove it in v2 if yes.

"The comments below" was referring to my comments on the patch, not the next
paragraph.  "don't address that question" refers to whether the kdump kernel
could be abused.

We're trying to close integrity gaps, not add new ones.  Verifying the UKI's
signature addresses the integrity of the initramfs.  What about the integrity of
the kdump initramfs (or for that matter the kexec initramfs)?  If the kdump
initramfs was signed, IMA would be able to verify it before the kexec.

As for the next paragraph, based on Coiby's response, please remove it.

thanks,

Mimi
> 
> > 
> > > > 
> > > > And Coiby also mentioned that for kdump kernel incorrect ima-policy
> > > > loaded
> > > > by systemd could cause kdump kernel hang, and it's possible the booting
> > > > process may be stopped by a strict, albeit syntax-correct policy and
> > > > users
> > > > can't log into the system to fix the policy. In these cases, allowing to
> > > > disable IMA is very helpful too for kdump kernel.
> > > > 
> > > > Hence add a knob ima=on|off here to allow people to disable IMA in kdump
> > > > kenrel if needed.
> > 
> > ^kernel
> 
> Will change.
> 
> > 
> > > > 
> > > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > > ---
> > > >  .../admin-guide/kernel-parameters.txt         |  5 +++++
> > > >  security/integrity/ima/ima_main.c             | 22 +++++++++++++++++++
> > > >  2 files changed, 27 insertions(+)
> > > > 
> > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > > > b/Documentation/admin-guide/kernel-parameters.txt
> > > > index d9fd26b95b34..762fb6ddcc24 100644
> > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > @@ -2202,6 +2202,11 @@
> > > >  			different crypto accelerators. This option can
> > > > be
> > > > used
> > > >  			to achieve best performance for particular HW.
> > > >  
> > > > +	ima=		[IMA] Enable or disable IMA
> > > > +			Format: { "off" | "on" }
> > > > +			Default: "on"
> > > > +			Note that this is only useful for kdump kernel.
> > 
> > Instead of "useful" I would prefer something clearer like "limited".
> 
> Makes sense, will change.
> 
> > 
> > > > +
> > > >  	init=		[KNL]
> > > >  			Format: <full_path>
> > > >  			Run specified binary instead of /sbin/init as
> > > > init
> > > > diff --git a/security/integrity/ima/ima_main.c
> > > > b/security/integrity/ima/ima_main.c
> > > > index f3e7ac513db3..07af5c6af138 100644
> > > > --- a/security/integrity/ima/ima_main.c
> > > > +++ b/security/integrity/ima/ima_main.c
> > > > @@ -27,6 +27,7 @@
> > > >  #include <linux/fs.h>
> > > >  #include <linux/iversion.h>
> > > >  #include <linux/evm.h>
> > > > +#include <linux/crash_dump.h>
> > > >  
> > > >  #include "ima.h"
> > > >  
> > > > @@ -38,11 +39,27 @@ int ima_appraise;
> > > >  
> > > >  int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> > > >  static int hash_setup_done;
> > > > +static int ima_disabled;
> > 
> > Like the ima_hash_algo variable definition above, ima_disabled should be
> > defined as __ro_after_init.
> 
> Will add __ro_after_init.
> 
> > 
> > > >  
> > > >  static struct notifier_block ima_lsm_policy_notifier = {
> > > >  	.notifier_call = ima_lsm_policy_change,
> > > >  };
> > > >  
> > > > +static int __init ima_setup(char *str)
> > > > +{
> > 
> > is_kdump_kernel() should also be called here, before the tests below. 
> > Something like:
> > 
> > +       if (!is_kdump_kernel()) {
> > +               pr_info("Warning ima setup option only permitted in kdump");
> > +               return 1;
> > +       }
> 
> Sure, will change as suggested.
> 
> > 
> > > > +	if (strncmp(str, "off", 3) == 0)
> > > > +		ima_disabled = 1;
> > > > +	else if (strncmp(str, "on", 2) == 0)
> > > > +		ima_disabled = 0;
> > > > +	else
> > > > +		pr_err("Invalid ima setup option: \"%s\" , please
> > > > specify
> > > > ima=on|off.", str);
> > > > +
> > > > +	return 1;
> > > > +}
> > > > +__setup("ima=", ima_setup);
> > > > +
> > > > +
> > > > +
> > 
> > Remove the extraneous blank line.
> 
> sure.
> 
> > 
> > > >  static int __init hash_setup(char *str)
> > > >  {
> > > >  	struct ima_template_desc *template_desc =
> > > > ima_template_desc_current();
> > > > @@ -1184,6 +1201,11 @@ static int __init init_ima(void)
> > > >  {
> > > >  	int error;
> > > >  
> > > > +	if (ima_disabled && is_kdump_kernel()) {
> > > > +		pr_info("IMA functionality is disabled");
> > > > +		return 0;
> > > > +	}
> > > > +
> > 
> > Even with the additional call to is_kdump_kernel() in ima_setup, please keep
> > the is_kdump_kernel() test here as well.  Even though the code is self
> > describing, please add a one line comment emphasizing disabling IMA is
> > limited
> > to kdump.
> 
> OK, will keep code here as this v1 is and add one line of comment at
> above.
> 
> Thanks again.
> 
> > 
> > > >  	ima_appraise_parse_cmdline();
> > > >  	ima_init_template_list();
> > > >  	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> > > > -- 
> > > > 2.41.0
> > > > 
> > > 
> > > 

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Baoquan He]

On 05/22/25 at 07:08am, Mimi Zohar wrote:
> On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
> > On 05/21/25 at 08:54am, Mimi Zohar wrote:
> > > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > > > CC kexec list.
> > > > 
> > > > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > > > > extra memory. It would be very helpful to allow IMA to be disabled for
> > > > > kdump kernel.
> > 
> > Thanks a lot for careufl reviewing and great suggestions.
> > 
> > > 
> > > The real question is not whether kdump needs "IMA", but whether not enabling
> > > IMA in the kdump kernel could be abused.  The comments below don't address
> > > that question but limit/emphasize, as much as possible, turning IMA off is
> > > limited to the kdump kernel.
> > 
> > Are you suggesting removing below paragraph from patch log because they
> > are redundant? I can remove it in v2 if yes.
> 
> "The comments below" was referring to my comments on the patch, not the next
> paragraph.  "don't address that question" refers to whether the kdump kernel
> could be abused.
> 
> We're trying to close integrity gaps, not add new ones.  Verifying the UKI's
> signature addresses the integrity of the initramfs.  What about the integrity of
> the kdump initramfs (or for that matter the kexec initramfs)?  If the kdump
> initramfs was signed, IMA would be able to verify it before the kexec.

Kdump initramfs could be generated each time when loading once change is
detected, e.g newer kernel, kdump config tuning. It's different than
UNI's normal initramfs. We don't need verify it as far as I know
according to discussion with UNI dev, so ima=off can be set by default
in kdump kernel. Even though one day that's really needed, ima=on|off is
a switch, not a hard code.

Add people woiking on kdump UKI to CC.

> 
> As for the next paragraph, based on Coiby's response, please remove it.

Got it, thanks.

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

On Tue, 2025-05-27 at 11:25 +0800, Pingfan Liu wrote

When responding to kernel mailing lists, please use plain text not Mime encoded.

> On Thu, May 22, 2025 at 10:52 PM Baoquan He <bhe@redhat.com> wrote:
> > On 05/22/25 at 07:08am, Mimi Zohar wrote:
> > > On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
> > > > On 05/21/25 at 08:54am, Mimi Zohar wrote:
> > > > > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > > > > > CC kexec list.
> > > > > > 
> > > > > > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > > > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will
> > > > > > > cost
> > > > > > > extra memory. It would be very helpful to allow IMA to be disabled
> > > > > > > for
> > > > > > > kdump kernel.
> > > > 
> > > > Thanks a lot for careufl reviewing and great suggestions.
> > > > 
> > > > > 
> > > > > The real question is not whether kdump needs "IMA", but whether not
> > > > > enabling
> > > > > IMA in the kdump kernel could be abused.  The comments below don't
> > > > > address
> > > > > that question but limit/emphasize, as much as possible, turning IMA
> > > > > off is
> > > > > limited to the kdump kernel.
> > > > 
> > > > Are you suggesting removing below paragraph from patch log because they
> > > > are redundant? I can remove it in v2 if yes.
> > > 
> > > "The comments below" was referring to my comments on the patch, not the
> > > next
> > > paragraph.  "don't address that question" refers to whether the kdump
> > > kernel
> > > could be abused.
> > > 
> > > We're trying to close integrity gaps, not add new ones.  Verifying the
> > > UKI's
> > > signature addresses the integrity of the initramfs.  What about the
> > > integrity of
> > > the kdump initramfs (or for that matter the kexec initramfs)?  If the
> > > kdump
> > > initramfs was signed, IMA would be able to verify it before the kexec.
> 
> IMHO, from the higher level, if there is a requirement on the integrity of the
> initramfs, it should take a similar approach as UKI. And the system admin can
> choose whether to disable the unsafe format loader or not.

Yes, that is a possibility, probably a good aim, but in the case of kexec/kdump
that isn't really necessary.  As filesystem(s) support xattrs, IMA policies
could be written in terms of "func=KEXEC_INITRAMFS_CHECK" to include the
initramfs.

> 
> This other thing is how to make a handy signature on initramfs? It is neither
> PE nor ELF.

IMA supports signatures stored in the security.ima xattr or as an appended
signatures.

Mimi

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Pingfan Liu]

On Tue, May 27, 2025 at 10:18 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2025-05-27 at 11:25 +0800, Pingfan Liu wrote
>
> When responding to kernel mailing lists, please use plain text not Mime encoded.
>
> > On Thu, May 22, 2025 at 10:52 PM Baoquan He <bhe@redhat.com> wrote:
> > > On 05/22/25 at 07:08am, Mimi Zohar wrote:
> > > > On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
> > > > > On 05/21/25 at 08:54am, Mimi Zohar wrote:
> > > > > > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > > > > > > CC kexec list.
> > > > > > >
> > > > > > > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > > > > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will
> > > > > > > > cost
> > > > > > > > extra memory. It would be very helpful to allow IMA to be disabled
> > > > > > > > for
> > > > > > > > kdump kernel.
> > > > >
> > > > > Thanks a lot for careufl reviewing and great suggestions.
> > > > >
> > > > > >
> > > > > > The real question is not whether kdump needs "IMA", but whether not
> > > > > > enabling
> > > > > > IMA in the kdump kernel could be abused.  The comments below don't
> > > > > > address
> > > > > > that question but limit/emphasize, as much as possible, turning IMA
> > > > > > off is
> > > > > > limited to the kdump kernel.
> > > > >
> > > > > Are you suggesting removing below paragraph from patch log because they
> > > > > are redundant? I can remove it in v2 if yes.
> > > >
> > > > "The comments below" was referring to my comments on the patch, not the
> > > > next
> > > > paragraph.  "don't address that question" refers to whether the kdump
> > > > kernel
> > > > could be abused.
> > > >
> > > > We're trying to close integrity gaps, not add new ones.  Verifying the
> > > > UKI's
> > > > signature addresses the integrity of the initramfs.  What about the
> > > > integrity of
> > > > the kdump initramfs (or for that matter the kexec initramfs)?  If the
> > > > kdump
> > > > initramfs was signed, IMA would be able to verify it before the kexec.
> >
> > IMHO, from the higher level, if there is a requirement on the integrity of the
> > initramfs, it should take a similar approach as UKI. And the system admin can
> > choose whether to disable the unsafe format loader or not.
>
> Yes, that is a possibility, probably a good aim, but in the case of kexec/kdump
> that isn't really necessary.  As filesystem(s) support xattrs, IMA policies
> could be written in terms of "func=KEXEC_INITRAMFS_CHECK" to include the
> initramfs.
>

Just aware that we have such a cool feature. Thanks!

I checked the code. IIUC, the relevant code has already been in the
kernel. And the thing left to do is to install an IMA policy, right?

But there are still two things to be considered.
-1.The UEFI partition is FAT format, which can not support xattr
-2. Just like in the UKI case, the kernel fd content is not necessary
for the kernel image itself. The initramfs fd can be used to pass some
extra data if we use a temp file as a package. So checking the
signatures at the initramfs level will block this usage


> >
> > This other thing is how to make a handy signature on initramfs? It is neither
> > PE nor ELF.
>
> IMA supports signatures stored in the security.ima xattr or as an appended
> signatures.
>

Good to know it.

Best Regards,

Pingfan

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

On Thu, 2025-05-29 at 12:13 +0800, Pingfan Liu wrote:
> On Tue, May 27, 2025 at 10:18 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Tue, 2025-05-27 at 11:25 +0800, Pingfan Liu wrote
> > > > > 
> > > > > 
> > > > > We're trying to close integrity gaps, not add new ones.  Verifying the
> > > > > UKI's
> > > > > signature addresses the integrity of the initramfs.  What about the
> > > > > integrity of
> > > > > the kdump initramfs (or for that matter the kexec initramfs)?  If the
> > > > > kdump
> > > > > initramfs was signed, IMA would be able to verify it before the kexec.
> > > 
> > > IMHO, from the higher level, if there is a requirement on the integrity of
> > > the
> > > initramfs, it should take a similar approach as UKI. And the system admin
> > > can
> > > choose whether to disable the unsafe format loader or not.
> > 
> > Yes, that is a possibility, probably a good aim, but in the case of
> > kexec/kdump
> > that isn't really necessary.  As filesystem(s) support xattrs, IMA policies
> > could be written in terms of "func=KEXEC_INITRAMFS_CHECK" to include the
> > initramfs.
> > 
> 
> Just aware that we have such a cool feature. Thanks!

> I checked the code. IIUC, the relevant code has already been in the
> kernel. And the thing left to do is to install an IMA policy, right?

Correct.  The problem up to now has been that the initramfs was created on the
fly on the target system, so it was impossible to remotely sign it by the
distro.

> 
> But there are still two things to be considered.
> -1.The UEFI partition is FAT format, which can not support xattr

The normal kexec/kdump kernel image and initramfs are stored in /boot, not the
UEFI partition.  Is that changing?

e.g. kexec -s -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname -
r`.img --reuse-cmdline

> -2. Just like in the UKI case, the kernel fd content is not necessary
> for the kernel image itself. The initramfs fd can be used to pass some
> extra data if we use a temp file as a package. So checking the
> signatures at the initramfs level will block this usage

Sorry I lost you here.  What exactly is included in the UKI signature?  What is
this initramfs fd extra data?  Is it included in the UKI signature?  Can you
point me to some documentation?

thanks,

Mimi
> > 

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Pingfan Liu]

On Thu, May 29, 2025 at 10:32 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2025-05-29 at 12:13 +0800, Pingfan Liu wrote:
> > On Tue, May 27, 2025 at 10:18 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Tue, 2025-05-27 at 11:25 +0800, Pingfan Liu wrote
> > > > > >
> > > > > >
> > > > > > We're trying to close integrity gaps, not add new ones.  Verifying the
> > > > > > UKI's
> > > > > > signature addresses the integrity of the initramfs.  What about the
> > > > > > integrity of
> > > > > > the kdump initramfs (or for that matter the kexec initramfs)?  If the
> > > > > > kdump
> > > > > > initramfs was signed, IMA would be able to verify it before the kexec.
> > > >
> > > > IMHO, from the higher level, if there is a requirement on the integrity of
> > > > the
> > > > initramfs, it should take a similar approach as UKI. And the system admin
> > > > can
> > > > choose whether to disable the unsafe format loader or not.
> > >
> > > Yes, that is a possibility, probably a good aim, but in the case of
> > > kexec/kdump
> > > that isn't really necessary.  As filesystem(s) support xattrs, IMA policies
> > > could be written in terms of "func=KEXEC_INITRAMFS_CHECK" to include the
> > > initramfs.
> > >
> >
> > Just aware that we have such a cool feature. Thanks!
>
> > I checked the code. IIUC, the relevant code has already been in the
> > kernel. And the thing left to do is to install an IMA policy, right?
>
> Correct.  The problem up to now has been that the initramfs was created on the
> fly on the target system, so it was impossible to remotely sign it by the
> distro.
>
> >
> > But there are still two things to be considered.
> > -1.The UEFI partition is FAT format, which can not support xattr
>
> The normal kexec/kdump kernel image and initramfs are stored in /boot, not the
> UEFI partition.  Is that changing?
>
I think that is the case if grub is used as a bootloader.

But officially, only FAT32 is supported in the UEFI specification. So
if a UEFI application (let's say systemd-boot) tries to load kernel
and initramfs, then boots into kernel, it can only expect to read
these files from FAT32 partition.

> e.g. kexec -s -l /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname -
> r`.img --reuse-cmdline
>
> > -2. Just like in the UKI case, the kernel fd content is not necessary
> > for the kernel image itself. The initramfs fd can be used to pass some
> > extra data if we use a temp file as a package. So checking the
> > signatures at the initramfs level will block this usage
>
> Sorry I lost you here.  What exactly is included in the UKI signature?  What is
> this initramfs fd extra data?  Is it included in the UKI signature?  Can you
> point me to some documentation?
>

Sorry for the awkward expression, let me clarify it.

It is a pity that these things are on the fly and there are no documents yet.

With the advent of UKI, which encapsulates the kernel, initramfs, and
cmdline into a single PE file, the original kexec_file_load syscall
schema no longer aligns with this design.

SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
                unsigned long, cmdline_len, const char __user *, cmdline_ptr,
                unsigned long, flags)

In particular, the kernel_fd parameter no longer refers to just the
kernel image alone.
And the same thing may happen to initrd_fd.

In essence, it means the redesign or re-explaining of the
kexec_file_load() interface, but for the time being, these things are
on the fly.

Thanks,

Pingfan

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Coiby Xu]

On Thu, May 22, 2025 at 07:08:04AM -0400, Mimi Zohar wrote:
>On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
>> On 05/21/25 at 08:54am, Mimi Zohar wrote:
>> > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
>> > > CC kexec list.
>> > >
>> > > On 05/16/25 at 07:39am, Baoquan He wrote:
>> > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
>> > > > extra memory. It would be very helpful to allow IMA to be disabled for
>> > > > kdump kernel.
>>
>> Thanks a lot for careufl reviewing and great suggestions.
>>
>> >
>> > The real question is not whether kdump needs "IMA", but whether not enabling
>> > IMA in the kdump kernel could be abused.  The comments below don't address
>> > that question but limit/emphasize, as much as possible, turning IMA off is
>> > limited to the kdump kernel.
>>
>> Are you suggesting removing below paragraph from patch log because they
>> are redundant? I can remove it in v2 if yes.
>
>"The comments below" was referring to my comments on the patch, not the next
>paragraph.  "don't address that question" refers to whether the kdump kernel
>could be abused.
>
>We're trying to close integrity gaps, not add new ones.  Verifying the UKI's
>signature addresses the integrity of the initramfs.  What about the integrity of
>the kdump initramfs (or for that matter the kexec initramfs)?  If the kdump
>initramfs was signed, IMA would be able to verify it before the kexec.

Hi Mimi,

I thought you were asking that the commit message should address the
question why disabling IMA should be limited to the kdump kernel. It
turns out I misunderstood your concern.

Currently there is no way provided to verify the kdump initramfs as a
whole file or to verify individual files in the kdump initramfs.

As you have already known, the kdump initramfs is always generated on
the fly and will be re-generated when the dumping target changes or
some important files change. We try to generate a minimal initramfs in
order to save memory. So yes, it's impossible to sign it as a whole file
beforehand. 

And since xattrs like security.ima are not supported in the kdump
initramfs, we have no way to use IMA to verify individual file's
integrity.  In fact, we have to stop IMA from working otherwise it's
very likely kdump will break.

So far, I'm not aware of any bug report that complains kdump stops
working because of IMA. So it indicates very few users are trying to use
IMA in kdump.

If users do have concerns on the integrity of kdump initramfs, I think
we can advice users to make sure the deployed IMA policy will verify the
integrity of the files while they are being collected and copied into
the kdump initramfs by tools like dracut.

-- 
Best regards,
Coiby

Re: [PATCH] ima: add a knob ima= to make IMA be able to be disabled

[Mimi Zohar]

On Wed, 2025-06-04 at 11:34 +0800, Coiby Xu wrote:
> On Thu, May 22, 2025 at 07:08:04AM -0400, Mimi Zohar wrote:
> > On Thu, 2025-05-22 at 11:24 +0800, Baoquan He wrote:
> > > On 05/21/25 at 08:54am, Mimi Zohar wrote:
> > > > On Fri, 2025-05-16 at 08:22 +0800, Baoquan He wrote:
> > > > > CC kexec list.
> > > > > 
> > > > > On 05/16/25 at 07:39am, Baoquan He wrote:
> > > > > > Kdump kernel doesn't need IMA functionality, and enabling IMA will cost
> > > > > > extra memory. It would be very helpful to allow IMA to be disabled for
> > > > > > kdump kernel.
> > > 
> > > Thanks a lot for careufl reviewing and great suggestions.
> > > 
> > > > 
> > > > The real question is not whether kdump needs "IMA", but whether not enabling
> > > > IMA in the kdump kernel could be abused.  The comments below don't address
> > > > that question but limit/emphasize, as much as possible, turning IMA off is
> > > > limited to the kdump kernel.
> > > 
> > > Are you suggesting removing below paragraph from patch log because they
> > > are redundant? I can remove it in v2 if yes.
> > 
> > "The comments below" was referring to my comments on the patch, not the next
> > paragraph.  "don't address that question" refers to whether the kdump kernel
> > could be abused.
> > 
> > We're trying to close integrity gaps, not add new ones.  Verifying the UKI's
> > signature addresses the integrity of the initramfs.  What about the integrity of
> > the kdump initramfs (or for that matter the kexec initramfs)?  If the kdump
> > initramfs was signed, IMA would be able to verify it before the kexec.
> 
> Hi Mimi,
> 
> I thought you were asking that the commit message should address the
> question why disabling IMA should be limited to the kdump kernel. It
> turns out I misunderstood your concern.
> 
> Currently there is no way provided to verify the kdump initramfs as a
> whole file or to verify individual files in the kdump initramfs.

There were multiple attempts to close this integrity gap, but none of them were
upstreamed.
> 
> As you have already known, the kdump initramfs is always generated on
> the fly and will be re-generated when the dumping target changes or
> some important files change. We try to generate a minimal initramfs in
> order to save memory. So yes, it's impossible to sign it as a whole file
> beforehand.

I'm just curious as to how UKI includes the initramfs, if it does, in the
signature.

> 
> And since xattrs like security.ima are not supported in the kdump
> initramfs, we have no way to use IMA to verify individual file's
> integrity.  In fact, we have to stop IMA from working otherwise it's
> very likely kdump will break.
> 
> So far, I'm not aware of any bug report that complains kdump stops
> working because of IMA. So it indicates very few users are trying to use
> IMA in kdump.
> 
> If users do have concerns on the integrity of kdump initramfs, I think
> we can advice users to make sure the deployed IMA policy will verify the
> integrity of the files while they are being collected and copied into
> the kdump initramfs by tools like dracut.

For now, I'd prefer to leave it as an integrity gap that still needs to be
addressed.

thanks,

Mimi