Re: [PATCH v3 2/9] ima: efi: Drop unnecessary check for CONFIG_MODULE_SIG/CONFIG_KEXEC_SIG

[Mimi Zohar <zohar@linux.ibm.com>]

On Wed, 2025-05-14 at 20:25 +0200, Thomas Weißschuh wrote:
> May 14, 2025 19:39:37 Mimi Zohar <zohar@linux.ibm.com>:
> 
> > On Wed, 2025-05-14 at 11:09 -0400, Mimi Zohar wrote:
> > > On Tue, 2025-04-29 at 15:04 +0200, Thomas Weißschuh wrote:
> > > > When configuration settings are disabled the guarded functions are
> > > > defined as empty stubs, so the check is unnecessary.
> > > > The specific configuration option for set_module_sig_enforced() is
> > > > about to change and removing the checks avoids some later churn.
> > > > 
> > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > > 
> > > > ---
> > > > This patch is not strictly necessary right now, but makes looking for
> > > > usages of CONFIG_MODULE_SIG easier.
> > > > ---
> > > >  security/integrity/ima/ima_efi.c | 6 ++----
> > > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
> > > > index
> > > > 138029bfcce1e40ef37700c15e30909f6e9b4f2d..a35dd166ad47beb4a7d46cc3e8fc604f57e03ecb
> > > > 100644
> > > > --- a/security/integrity/ima/ima_efi.c
> > > > +++ b/security/integrity/ima/ima_efi.c
> > > > @@ -68,10 +68,8 @@ static const char * const sb_arch_rules[] = {
> > > >  const char * const *arch_get_ima_policy(void)
> > > >  {
> > > >     if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> > > > -       if (IS_ENABLED(CONFIG_MODULE_SIG))
> > > > -           set_module_sig_enforced();
> > > > -       if (IS_ENABLED(CONFIG_KEXEC_SIG))
> > > > -           set_kexec_sig_enforced();
> > > > +       set_module_sig_enforced();
> > > > +       set_kexec_sig_enforced();
> > > >         return sb_arch_rules;
> > > 
> > > Hi Thomas,
> > > 
> > > I'm just getting to looking at this patch set.  Sorry for the delay.
> > > 
> > > Testing whether CONFIG_MODULE_SIG and CONFIG_KEXEC_SIG are configured gives priority
> > > to them, rather than to the IMA support.  Without any other changes, both signature
> > > verifications would be enforced.  Is that the intention?
> > 
> > Never mind, got it.
> > 
> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> 
> Thanks for the review!
> 
> Given that this series has no chance
> of getting into the next merge window,
> would it be possible to take the two IMA preparation patches
> through the IMA tree to have them out of the way?

I'm fine with picking up the two patches simply as code cleanup, meaning dropping the last
sentence of the patch description, after some testing.

Mimi