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

[Baoquan He <bhe@redhat.com>]

Hi Paul,

On 03/31/25 at 08:22am, Paul Menzel wrote:
> 
Thanks for your careful reviewing.
> 
> Thank you for your patch. I’d add the knob name to the commit message
> summary/title, so it shows up in `git log --oneline`.

Sounds great, will change.

> 
> Am 31.03.25 um 08:16 schrieb Baoquan He:
> > It doesn't make sense to run IMA functionality in kdump kernel, and that
> > will cost extra memory. It would be great to allow IMA to be disabled on
> > purpose, e.g for kdump kernel.
> > 
> > Hence add a knob here to allow people to disable IMA if needed.
> 
> `initcall_blacklist=…` could be used already. I prefer a dedicated parameter
> too though.

Yes, adding parameter can provide an explicit functionality to the
feature. While 'initcall_blacklist=' can't guarantee there won't be
dependency or connection between ima and other feature, and people could
add or change the connection anytime when userspace is using it but not
knowing the change.

> 
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> >   security/integrity/ima/ima_main.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 28b8b0db6f9b..5d677d1389fe 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -38,11 +38,27 @@ int ima_appraise;
> >   int __ro_after_init ima_hash_algo = HASH_ALGO_SHA1;
> >   static int hash_setup_done;
> > +static int ima_disabled = 0;
> >   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\" ", str);
> 
> I’d add the allowed strings.

Sounds better, will change.

> 
> > +
> > +	return 1;
> > +}
> > +__setup("ima=", ima_setup);
> > +
> > +
> > +
> >   static int __init hash_setup(char *str)
> >   {
> >   	struct ima_template_desc *template_desc = ima_template_desc_current();
> > @@ -1176,6 +1192,11 @@ static int __init init_ima(void)
> >   {
> >   	int error;
> > +	if (ima_disabled) {
> > +		pr_info("IMA functionality is disabled on purpose!");
> 
> … on Linux CLI.

I may not get the suggestion in this place, could you be more specific?

> 
> > +		return 0;
> > +	}
> > +
> >   	ima_appraise_parse_cmdline();
> >   	ima_init_template_list();
> >   	hash_setup(CONFIG_IMA_DEFAULT_HASH);
> 
> 
> Kind regards,
> 
> Paul
>