Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module

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

On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
> On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
> > 
> > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> > is enabled, IMA has no way to verify the appended module signature as it
> > can't decompress the module.
> > 
> > Define a new LSM hook security_kernel_module_read_file which will be
> > called after kernel module decompression is done so IMA can access the
> > decompressed kernel module to verify the appended signature.
> > 
> > Since IMA can access both xattr and appended kernel module signature
> > with the new LSM hook, it no longer uses the security_kernel_post_read_file
> > LSM hook for kernel module loading.
> > 
> > Before enabling in-kernel module decompression, a kernel module in
> > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> > kernel module rule in secure_boot policy to allow either an IMA
> > signature OR an appended signature i.e. to use
> > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
> > 
> > Reported-by: Karel Srot <ksrot@redhat.com>
> > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > ---
> > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
> > 
> >  include/linux/lsm_hook_defs.h       |  2 ++
> >  include/linux/security.h            |  7 +++++++
> >  kernel/module/main.c                | 10 +++++++++-
> >  security/integrity/ima/ima_main.c   | 26 ++++++++++++++++++++++++++
> >  security/integrity/ima/ima_policy.c |  2 +-
> >  security/security.c                 | 17 +++++++++++++++++
> >  6 files changed, 62 insertions(+), 2 deletions(-)
> 
> We don't really need a new LSM hook for this do we?  Can't we just
> define a new file read type, e.g.  READING_MODULE_DECOMPRESS, and do
> another call to security_kernel_post_read_file() after the module is
> unpacked?  Something like the snippet below ...

Yes, this is similar to my suggestion based on defining multiple enumerations:
READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE. 
With this solution, IMA would need to make an exception in the post kernel
module read for the READING_COMPRESSED_MODULE case, since the kernel module has
not yet been decompressed.

Coiby suggested further simplification by moving the call later.  At which point
either there is or isn't an appended signature for non-compressed and
decompressed kernel modules.

As long as you don't have a problem calling the security_kernel_post_read_file()
hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?

> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..f127000d2e0a 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
> ar __user * uargs, int
>                        mod_stat_add_long(len, &invalid_decompress_bytes);
>                        return err;
>                }
> +
> +               err = security_kernel_post_read_file(f,
> +                                                    (char *)info.hdr, info.len,
> +                                                    READING_MODULE_DECOMPRESS);
> +               if (err) {
> +                       mod_stat_inc(&failed_kreads);
> +                       return err;
> +               }
>        } else {
>                info.hdr = buf;
>                info.len = len;

== defer security_kernel_post_read_file() call to here ==

Mimi