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

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

[Coiby Xu]

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(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..ced42eb8b618 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -232,6 +232,8 @@ LSM_HOOK(int, 0, kernel_read_file, struct file *file,
 	 enum kernel_read_file_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_read_file, struct file *file, char *buf,
 	 loff_t size, enum kernel_read_file_id id)
+LSM_HOOK(int, 0, kernel_module_read_file, struct file *file, char *buf,
+	 loff_t size)
 LSM_HOOK(int, 0, task_fix_setuid, struct cred *new, const struct cred *old,
 	 int flags)
 LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old,
diff --git a/include/linux/security.h b/include/linux/security.h
index 92ac3f27b973..e47951292c73 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -508,6 +508,7 @@ int security_kernel_read_file(struct file *file, enum kernel_read_file_id id,
 			      bool contents);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
+int security_kernel_module_read_file(struct file *file, char *buf, loff_t size);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
 			     int flags);
 int security_task_fix_setgid(struct cred *new, const struct cred *old,
@@ -1295,6 +1296,12 @@ static inline int security_kernel_post_read_file(struct file *file,
 	return 0;
 }
 
+static inline int security_kernel_module_read_file(struct file *file,
+						   char *buf, loff_t size)
+{
+	return 0;
+}
+
 static inline int security_task_fix_setuid(struct cred *new,
 					   const struct cred *old,
 					   int flags)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..40bc86fa7384 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 	struct load_info info = { };
 	void *buf = NULL;
 	int len;
+	int err;
 
 	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
 	if (len < 0) {
@@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 	}
 
 	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+		err = module_decompress(&info, buf, len);
 		vfree(buf); /* compressed data is no longer needed */
 		if (err) {
 			mod_stat_inc(&failed_decompress);
@@ -3698,6 +3699,13 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 		info.len = len;
 	}
 
+	err = security_kernel_module_read_file(f, (char *)info.hdr, info.len);
+	if (err) {
+		mod_stat_inc(&failed_kreads);
+		free_copy(&info, flags);
+		return err;
+	}
+
 	return load_module(&info, uargs, flags);
 }
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cdd225f65a62..53d2e90176ea 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -635,6 +635,27 @@ static int ima_file_check(struct file *file, int mask)
 					   MAY_APPEND), FILE_CHECK);
 }
 
+/**
+ * ima_read_kernel_module - collect/appraise/audit measurement
+ * @file: file pointer to the module.
+ * @buf: buffer containing module data (possibly decompressed).
+ * @size: size of the buffer.
+ *
+ * This IMA hook for kernel_module_read_file LSM hook is called after a kernel
+ * module has been read into memory and (if applicable) decompressed. It
+ * measures and/or appraises the module based on the IMA policy.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int ima_read_kernel_module(struct file *file, char *buf, loff_t size)
+{
+	struct lsm_prop prop;
+
+	security_current_getlsmprop_subj(&prop);
+	return process_measurement(file, current_cred(), &prop, buf, size,
+				   MAY_READ, MODULE_CHECK);
+}
+
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
 			    size_t buf_size)
 {
@@ -881,6 +902,10 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
 	enum ima_hooks func;
 	struct lsm_prop prop;
 
+	/* kernel module will be addressed in ima_read_kernel_module */
+	if (read_id == READING_MODULE)
+		return 0;
+
 	/* permit signed certs */
 	if (!file && read_id == READING_X509_CERTIFICATE)
 		return 0;
@@ -1250,6 +1275,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(kernel_load_data, ima_load_data),
 	LSM_HOOK_INIT(kernel_post_load_data, ima_post_load_data),
 	LSM_HOOK_INIT(kernel_read_file, ima_read_file),
+	LSM_HOOK_INIT(kernel_module_read_file, ima_read_kernel_module),
 	LSM_HOOK_INIT(kernel_post_read_file, ima_post_read_file),
 	LSM_HOOK_INIT(path_post_mknod, ima_post_path_mknod),
 #ifdef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..2c9bdc618ac9 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -241,7 +241,7 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED | IMA_CHECK_BLACKLIST},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/security.c b/security/security.c
index 4d3c03a4524c..311ba63a8889 100644
--- a/security/security.c
+++ b/security/security.c
@@ -3442,6 +3442,23 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 }
 EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
 
+/**
+ * security_kernel_module_read_file() - Read a kernel module loaded by finit_module
+ * @file: file
+ * @buf: contents of decompressed kernel module
+ * @size: size of decompressed kernel module
+ *
+ * Read a kernel module loaded by the finit_module syscall. Unlike
+ * security_kernel_post_read_file, it has access to the decompressed kernel module.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_kernel_module_read_file(struct file *file, char *buf, loff_t size)
+{
+	return call_int_hook(kernel_module_read_file, file, buf, size);
+}
+EXPORT_SYMBOL_GPL(security_kernel_module_read_file);
+
 /**
  * security_kernel_load_data() - Load data provided by userspace
  * @id: data identifier

base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
-- 
2.51.0

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

[Paul Moore]

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 ...

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;

-- 
paul-moore.com

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

[Mimi Zohar]

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

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

[Paul Moore]

On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> 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?

It isn't clear from these comments if you are talking about moving
only the second security_kernel_post_read_file() call that was
proposed for init_module_from_file() to later in the function, leaving
the call in kernel_read_file() intact, or something else?

I think we want to leave the hook calls in kernel_read_file() intact,
in which case I'm not certain what advantage there is in moving the
security_kernel_post_read_file() call to a location where it is called
in init_module_from_file() regardless of if the module is compressed
or not.  In the uncompressed case you are calling the hook twice for
no real benefit?  It may be helpful to submit a patch with your
proposal as a patch can be worth a thousand words ;)

> > 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 ==

-- 
paul-moore.com

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

[Coiby Xu]

On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
>On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> 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?
>
>It isn't clear from these comments if you are talking about moving
>only the second security_kernel_post_read_file() call that was
>proposed for init_module_from_file() to later in the function, leaving
>the call in kernel_read_file() intact, or something else?

Hi Paul and Mimi,

Thanks for sharing your feedback! Yes, you are right, there is no need
for a new LSM hook. Actually by not introducing a new LSM hook, we can
have a much simpler solution!

>
>I think we want to leave the hook calls in kernel_read_file() intact,
>in which case I'm not certain what advantage there is in moving the
>security_kernel_post_read_file() call to a location where it is called
>in init_module_from_file() regardless of if the module is compressed
>or not.  In the uncompressed case you are calling the hook twice for
>no real benefit?  It may be helpful to submit a patch with your
>proposal as a patch can be worth a thousand words ;)
>
>
>> > 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 ==

By moving security_kernel_post_read_file, I think what Mimi means is to
move security_kernel_post_read_file in init_module_from_file() to later
in the function,

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b261849362a..66725e53fef0c1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
  	struct load_info info = { };
  	void *buf = NULL;
  	int len;
+	int err;
  
  	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
  	if (len < 0) {
@@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
  	}
  
  	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+		err = module_decompress(&info, buf, len);
  		vfree(buf); /* compressed data is no longer needed */
  		if (err) {
  			mod_stat_inc(&failed_decompress);
@@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
  		info.len = len;
  	}
  
+	err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+					     READING_MODULE);
+	if (err) {
+		mod_stat_inc(&failed_kreads);
+		free_copy(&info, flags);
+		return err;
+	}
+
  	return load_module(&info, uargs, flags);
  }

If we only call security_kernel_post_read_file the 2nd time for a
decompressed kernel module, IMA won't be sure what to do when
security_kernel_post_read_file is called for the 1st time because it
can't distinguish between a compressed module with appended signature or
a uncompressed module without appended signature. If it permits 1st
calling security_kernel_post_read_file, a uncompressed module without
appended signature can be loaded. If it doesn't permit 1st calling
security_kernel_post_read_file, there is no change to call
security_kernel_post_read_file again for decompressed module.

And you are right, there is no need to call
security_kernel_post_read_file twice. And from the perspective of IMA,
it simplifies reasoning if it is guaranteed that IMA will always access
uncompressed kernel module regardless regardless of its original
compression state. 

So I think a better solution is to stop calling
security_kernel_post_read_file in kernel_read_file for READING_MODULE.
This can also avoiding introducing an unnecessary
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
can make the solution even simpler,

diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index de32c95d823dbd..7c78e84def6ec7 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
  			goto out_free;
  		}
  
-		ret = security_kernel_post_read_file(file, *buf, i_size, id);
+		/*
+		 * security_kernel_post_read_file will be called later after
+		 * a read kernel module is truly decompressed
+		 */
+		if (id != READING_MODULE)
+			ret = security_kernel_post_read_file(file, *buf, i_size, id);
  	}

Btw, I notice IMA is the only user of security_kernel_post_read_file so
this change won't affect other LSMs. For a full patch, please visit
https://github.com/coiby/linux/commit/558d85779ab5d794874749ecfae0e48b890bf3e0.patch

If there are concerns that I'm unaware of and a new
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration is
necessary, here's another patch
https://github.com/coiby/linux/commit/cdd40317b6070f48ec871c6a89428084f38ca083.patch


>
>-- 
>paul-moore.com
>

-- 
Best regards,
Coiby

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

[Paul Moore]

On Tue, Nov 4, 2025 at 7:19 PM Coiby Xu <coxu@redhat.com> wrote:
> On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
> >On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >> 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?
> >
> >It isn't clear from these comments if you are talking about moving
> >only the second security_kernel_post_read_file() call that was
> >proposed for init_module_from_file() to later in the function, leaving
> >the call in kernel_read_file() intact, or something else?
>
> Hi Paul and Mimi,
>
> Thanks for sharing your feedback! Yes, you are right, there is no need
> for a new LSM hook. Actually by not introducing a new LSM hook, we can
> have a much simpler solution!
>
> >
> >I think we want to leave the hook calls in kernel_read_file() intact,
> >in which case I'm not certain what advantage there is in moving the
> >security_kernel_post_read_file() call to a location where it is called
> >in init_module_from_file() regardless of if the module is compressed
> >or not.  In the uncompressed case you are calling the hook twice for
> >no real benefit?  It may be helpful to submit a patch with your
> >proposal as a patch can be worth a thousand words ;)
> >
> >
> >> > 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 ==
>
> By moving security_kernel_post_read_file, I think what Mimi means is to
> move security_kernel_post_read_file in init_module_from_file() to later
> in the function,
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362a..66725e53fef0c1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>         struct load_info info = { };
>         void *buf = NULL;
>         int len;
> +       int err;
>
>         len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
>         if (len < 0) {
> @@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>         }
>
>         if (flags & MODULE_INIT_COMPRESSED_FILE) {
> -               int err = module_decompress(&info, buf, len);
> +               err = module_decompress(&info, buf, len);
>                 vfree(buf); /* compressed data is no longer needed */
>                 if (err) {
>                         mod_stat_inc(&failed_decompress);
> @@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>                 info.len = len;
>         }
>
> +       err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
> +                                            READING_MODULE);
> +       if (err) {
> +               mod_stat_inc(&failed_kreads);
> +               free_copy(&info, flags);
> +               return err;
> +       }
> +
>         return load_module(&info, uargs, flags);
>   }
>
> If we only call security_kernel_post_read_file the 2nd time for a
> decompressed kernel module, IMA won't be sure what to do when
> security_kernel_post_read_file is called for the 1st time because it
> can't distinguish between a compressed module with appended signature or
> a uncompressed module without appended signature. If it permits 1st
> calling security_kernel_post_read_file, a uncompressed module without
> appended signature can be loaded. If it doesn't permit 1st calling
> security_kernel_post_read_file, there is no change to call
> security_kernel_post_read_file again for decompressed module.
>
> And you are right, there is no need to call
> security_kernel_post_read_file twice. And from the perspective of IMA,
> it simplifies reasoning if it is guaranteed that IMA will always access
> uncompressed kernel module regardless regardless of its original
> compression state.
>
> So I think a better solution is to stop calling
> security_kernel_post_read_file in kernel_read_file for READING_MODULE.
> This can also avoiding introducing an unnecessary
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
> can make the solution even simpler,
>
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index de32c95d823dbd..7c78e84def6ec7 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
>                         goto out_free;
>                 }
>
> -               ret = security_kernel_post_read_file(file, *buf, i_size, id);
> +               /*
> +                * security_kernel_post_read_file will be called later after
> +                * a read kernel module is truly decompressed
> +                */
> +               if (id != READING_MODULE)
> +                       ret = security_kernel_post_read_file(file, *buf, i_size, id);
>         }

Assuming I'm understanding the problem correctly, I think you're
making this harder than it needs to be.  I believe something like this
should solve the problem without having to add more conditionals
around the hooks in kernel_read_file(), and limiting the multiple
security_kernel_post_read_file() calls to just the compressed case ...
and honestly in each of the _post_read_file() calls in the compressed
case, the buffer contents have changed so it somewhat makes sense.

Given the code below, IMA could simply ignore the
READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
in that case) and focus on the READING_MODULE case as it does today.
I expect the associated IMA patch would be both trivial and small.

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..b435c498ec01 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
ent *u)

static int init_module_from_file(struct file *f, const char __user * uargs, int
flags)
{
+       bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
       struct load_info info = { };
       void *buf = NULL;
       int len;

-       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+                              compressed ? READING_MODULE_COMPRESSED : READING_
MODULE);
       if (len < 0) {
               mod_stat_inc(&failed_kreads);
               return len;
       }

-       if (flags & MODULE_INIT_COMPRESSED_FILE) {
+       if (compressed) {
               int err = module_decompress(&info, buf, len);
               vfree(buf); /* compressed data is no longer needed */
               if (err) {
@@ -3693,6 +3695,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);
+               if (err) {
+                       mod_stat_inc(&failed_kreads);
+                       return err;
+               }
       } else {
               info.hdr = buf;
               info.len = len;

-- 
paul-moore.com

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

[Mimi Zohar]

On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> Assuming I'm understanding the problem correctly, I think you're
> making this harder than it needs to be.  I believe something like this
> should solve the problem without having to add more conditionals
> around the hooks in kernel_read_file(), and limiting the multiple
> security_kernel_post_read_file() calls to just the compressed case ...
> and honestly in each of the _post_read_file() calls in the compressed
> case, the buffer contents have changed so it somewhat makes sense.

> Given the code below, IMA could simply ignore the
> READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> in that case) and focus on the READING_MODULE case as it does today.
> I expect the associated IMA patch would be both trivial and small.
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..b435c498ec01 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> ent *u)
> 
> static int init_module_from_file(struct file *f, const char __user * uargs, int
> flags)
> {
> +       bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
>        struct load_info info = { };
>        void *buf = NULL;
>        int len;
> 
> -       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> +       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> +                              compressed ? READING_MODULE_COMPRESSED : READING_
> MODULE);
>        if (len < 0) {
>                mod_stat_inc(&failed_kreads);
>                return len;
>        }
> 
> -       if (flags & MODULE_INIT_COMPRESSED_FILE) {
> +       if (compressed) {
>                int err = module_decompress(&info, buf, len);
>                vfree(buf); /* compressed data is no longer needed */
>                if (err) {
> @@ -3693,6 +3695,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);

Without changing the enumeration here, IMA would not be able to differentiate
the first call to security_kernel_post_read_file() and this one.  The first call
would result in unnecessary error messages.

Adding an additional call to security_kernel_post_read_file() here, would
require defining 2 additional enumerations: READING_MODULE_COMPRESSED,
READING_MODULE_DECOMPRESSED.

> +               if (err) {
> +                       mod_stat_inc(&failed_kreads);
> +                       return err;
> +               }
>        } else {
>                info.hdr = buf;
>                info.len = len;

Deferring the security_kernel_post_read_file() call to here, eliminates the need
for defining additional enumerations.  (Coiby's first link.)

Adding an additional call to security_kernel_post_read_file() here, requires 1
additional enumeration.  (Coiby's 2nd link.)

Mimi

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

[Paul Moore]

On Wed, Nov 5, 2025 at 9:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> > Assuming I'm understanding the problem correctly, I think you're
> > making this harder than it needs to be.  I believe something like this
> > should solve the problem without having to add more conditionals
> > around the hooks in kernel_read_file(), and limiting the multiple
> > security_kernel_post_read_file() calls to just the compressed case ...
> > and honestly in each of the _post_read_file() calls in the compressed
> > case, the buffer contents have changed so it somewhat makes sense.
>
> > Given the code below, IMA could simply ignore the
> > READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> > in that case) and focus on the READING_MODULE case as it does today.
> > I expect the associated IMA patch would be both trivial and small.
> >
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index c66b26184936..b435c498ec01 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> > ent *u)
> >
> > static int init_module_from_file(struct file *f, const char __user * uargs, int
> > flags)
> > {
> > +       bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> >        struct load_info info = { };
> >        void *buf = NULL;
> >        int len;
> >
> > -       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> > +       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> > +                              compressed ? READING_MODULE_COMPRESSED : READING_
> > MODULE);
> >        if (len < 0) {
> >                mod_stat_inc(&failed_kreads);
> >                return len;
> >        }
> >
> > -       if (flags & MODULE_INIT_COMPRESSED_FILE) {
> > +       if (compressed) {
> >                int err = module_decompress(&info, buf, len);
> >                vfree(buf); /* compressed data is no longer needed */
> >                if (err) {
> > @@ -3693,6 +3695,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);
>
> Without changing the enumeration here, IMA would not be able to differentiate
> the first call to security_kernel_post_read_file() and this one.  The first call
> would result in unnecessary error messages.

Given the patch snippet above, in the case where an uncompressed
module is passed into init_module_from_file() there would be the
following checks, in this order:

 * kernel_read_file()
 -> security_kernel_read_file(READING_MODULE)
 -> security_kernel_post_read_file(READING_MODULE)
 * init_module_from_file()
 -> NONE

... this should be the same as the current behavior.

In the case where a compressed module is passed into
init_module_from_file() there would be the following checks, in this
order:

 * kernel_read_file()
 -> security_kernel_read_file(READING_MODULE_COMPRESSED)
 -> security_kernel_post_read_file(READING_MODULE_COMPRESSED)
 * init_module_from_file()
 -> security_kernel_post_read_file(READING_MODULE)

... the two differences being that the hook calls in
kernel_read_file() use the READING_MODULE_COMPRESSED id, which seems
appropriate as the data passed to the hook is the compressed
representation, and the additional _post_read_file() hook call in
init_module_from_file() using the READING_MODULE id, as the data
passed to the hook is now uncompressed.  Not only should IMA be able
to easily differentiate between the two _post_read_file() calls, but
it should have access to both the compressed and uncompressed data.

-- 
paul-moore.com

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

[Mimi Zohar]

On Wed, 2025-11-05 at 10:42 -0500, Paul Moore wrote:
> On Wed, Nov 5, 2025 at 9:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> > > Assuming I'm understanding the problem correctly, I think you're
> > > making this harder than it needs to be.  I believe something like this
> > > should solve the problem without having to add more conditionals
> > > around the hooks in kernel_read_file(), and limiting the multiple
> > > security_kernel_post_read_file() calls to just the compressed case ...
> > > and honestly in each of the _post_read_file() calls in the compressed
> > > case, the buffer contents have changed so it somewhat makes sense.
> > 
> > > Given the code below, IMA could simply ignore the
> > > READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> > > in that case) and focus on the READING_MODULE case as it does today.
> > > I expect the associated IMA patch would be both trivial and small.
> > > 
> > > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > > index c66b26184936..b435c498ec01 100644
> > > --- a/kernel/module/main.c
> > > +++ b/kernel/module/main.c
> > > @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> > > ent *u)
> > > 
> > > static int init_module_from_file(struct file *f, const char __user * uargs, int
> > > flags)
> > > {
> > > +       bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> > >        struct load_info info = { };
> > >        void *buf = NULL;
> > >        int len;
> > > 
> > > -       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> > > +       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> > > +                              compressed ? READING_MODULE_COMPRESSED : READING_
> > > MODULE);
> > >        if (len < 0) {
> > >                mod_stat_inc(&failed_kreads);
> > >                return len;
> > >        }
> > > 
> > > -       if (flags & MODULE_INIT_COMPRESSED_FILE) {
> > > +       if (compressed) {
> > >                int err = module_decompress(&info, buf, len);
> > >                vfree(buf); /* compressed data is no longer needed */
> > >                if (err) {
> > > @@ -3693,6 +3695,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);
> > 
> > Without changing the enumeration here, IMA would not be able to differentiate
> > the first call to security_kernel_post_read_file() and this one.  The first call
> > would result in unnecessary error messages.
> 
> Given the patch snippet above, in the case where an uncompressed
> module is passed into init_module_from_file() there would be the
> following checks, in this order:
> 
>  * kernel_read_file()
>  -> security_kernel_read_file(READING_MODULE)
>  -> security_kernel_post_read_file(READING_MODULE)
>  * init_module_from_file()
>  -> NONE
> 
> ... this should be the same as the current behavior.
> 
> In the case where a compressed module is passed into
> init_module_from_file() there would be the following checks, in this
> order:
> 
>  * kernel_read_file()
>  -> security_kernel_read_file(READING_MODULE_COMPRESSED)
>  -> security_kernel_post_read_file(READING_MODULE_COMPRESSED)
>  * init_module_from_file()
>  -> security_kernel_post_read_file(READING_MODULE)
> 
> ... the two differences being that the hook calls in
> kernel_read_file() use the READING_MODULE_COMPRESSED id, which seems
> appropriate as the data passed to the hook is the compressed
> representation, and the additional _post_read_file() hook call in
> init_module_from_file() using the READING_MODULE id, as the data
> passed to the hook is now uncompressed.  Not only should IMA be able
> to easily differentiate between the two _post_read_file() calls, but
> it should have access to both the compressed and uncompressed data.

Thanks, Paul.  Yes, a single additional enumeration is enough.

Mimi

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

[Mimi Zohar]

On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
> > On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > 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?
> > 
> > It isn't clear from these comments if you are talking about moving
> > only the second security_kernel_post_read_file() call that was
> > proposed for init_module_from_file() to later in the function, leaving
> > the call in kernel_read_file() intact, or something else?
> 
> Hi Paul and Mimi,
> 
> Thanks for sharing your feedback! Yes, you are right, there is no need
> for a new LSM hook. Actually by not introducing a new LSM hook, we can
> have a much simpler solution!
> 
> > 
> > I think we want to leave the hook calls in kernel_read_file() intact,
> > in which case I'm not certain what advantage there is in moving the
> > security_kernel_post_read_file() call to a location where it is called
> > in init_module_from_file() regardless of if the module is compressed
> > or not.  In the uncompressed case you are calling the hook twice for
> > no real benefit?  It may be helpful to submit a patch with your
> > proposal as a patch can be worth a thousand words ;)
> > 
> > 
> > > > 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 ==
> 
> By moving security_kernel_post_read_file, I think what Mimi means is to
> move security_kernel_post_read_file in init_module_from_file() to later
> in the function,
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362a..66725e53fef0c1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>   	struct load_info info = { };
>   	void *buf = NULL;
>   	int len;
> +	int err;
>   
>   	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
>   	if (len < 0) {
> @@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>   	}
>   
>   	if (flags & MODULE_INIT_COMPRESSED_FILE) {
> -		int err = module_decompress(&info, buf, len);
> +		err = module_decompress(&info, buf, len);
>   		vfree(buf); /* compressed data is no longer needed */
>   		if (err) {
>   			mod_stat_inc(&failed_decompress);
> @@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
>   		info.len = len;
>   	}
>   
> +	err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
> +					     READING_MODULE);
> +	if (err) {
> +		mod_stat_inc(&failed_kreads);
> +		free_copy(&info, flags);
> +		return err;
> +	}
> +
>   	return load_module(&info, uargs, flags);
>   }
> 
> If we only call security_kernel_post_read_file the 2nd time for a
> decompressed kernel module, IMA won't be sure what to do when
> security_kernel_post_read_file is called for the 1st time because it
> can't distinguish between a compressed module with appended signature or
> a uncompressed module without appended signature. If it permits 1st
> calling security_kernel_post_read_file, a uncompressed module without
> appended signature can be loaded. If it doesn't permit 1st calling
> security_kernel_post_read_file, there is no change to call
> security_kernel_post_read_file again for decompressed module.
> 
> And you are right, there is no need to call
> security_kernel_post_read_file twice. And from the perspective of IMA,
> it simplifies reasoning if it is guaranteed that IMA will always access
> uncompressed kernel module regardless regardless of its original
> compression state. 
> 
> So I think a better solution is to stop calling
> security_kernel_post_read_file in kernel_read_file for READING_MODULE.
> This can also avoiding introducing an unnecessary
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
> can make the solution even simpler,
> 
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index de32c95d823dbd..7c78e84def6ec7 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
>   			goto out_free;
>   		}
>   
> -		ret = security_kernel_post_read_file(file, *buf, i_size, id);
> +		/*
> +		 * security_kernel_post_read_file will be called later after
> +		 * a read kernel module is truly decompressed
> +		 */
> +		if (id != READING_MODULE)
> +			ret = security_kernel_post_read_file(file, *buf, i_size, id);
>   	}
> 
> Btw, I notice IMA is the only user of security_kernel_post_read_file so
> this change won't affect other LSMs. For a full patch, please visit
> https://github.com/coiby/linux/commit/558d85779ab5d794874749ecfae0e48b890bf3e0.patch
> 
> If there are concerns that I'm unaware of and a new
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration is
> necessary, here's another patch
> https://github.com/coiby/linux/commit/cdd40317b6070f48ec871c6a89428084f38ca083.patch

Hi Coiby,

Based on the conversation with Paul, there is no reason to remove the existing
security_kernel_post_read_file() call.

The changes are similar to the 2nd link, but a bit different.
- Define a single enumeration named READING_MODULE_COMPRESSED.

- In module/main.c add a new security_kernel_post_read_file() call immediately
after decompressing the kernel module.  Like a previous version of this patch,
call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
based on MODULE_INIT_COMPRESSED_FILE.

- In ima_post_read_file() defer verifying the signature when the enumeration is
READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)

thanks,

Mimi

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

[Coiby Xu]

On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
>On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
[...]
>
>Hi Coiby,
>
>Based on the conversation with Paul, there is no reason to remove the existing
>security_kernel_post_read_file() call.
>
>The changes are similar to the 2nd link, but a bit different.
>- Define a single enumeration named READING_MODULE_COMPRESSED.
>
>- In module/main.c add a new security_kernel_post_read_file() call immediately
>after decompressing the kernel module.  Like a previous version of this patch,
>call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
>based on MODULE_INIT_COMPRESSED_FILE.
>
>- In ima_post_read_file() defer verifying the signature when the enumeration is
>READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)

Hi Mimi,

Thanks for summarizing your conversation with Paul! I can confirm Paul's
approach works
https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul

While testing the patch today, I realized there is another
issue/challenge introduced by in-kernel module decompression. IMA
appraisal is to verify the digest of compressed kernel module but
currently the passed buffer is uncompressed module. When IMA uses
uncompressed module data to calculate the digest, xattr signature
verification will fail. If we always make IMA read the original kernel
module data again to calculate the digest, does it look like a
quick-and-dirty fix? If we can assume people won't load kernel module so
often, the performance impact is negligible. Otherwise we may have to
introduce a new LSM hook so IMA can access uncompressed and original
module data one time.

>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby

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

[Coiby Xu]

On Wed, Nov 05, 2025 at 03:25:05PM -0500, Mimi Zohar wrote:
>On Wed, 2025-11-05 at 10:42 -0500, Paul Moore wrote:
>> On Wed, Nov 5, 2025 at 9:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> > On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
>> > > Assuming I'm understanding the problem correctly, I think you're
>> > > making this harder than it needs to be.  I believe something like this
>> > > should solve the problem without having to add more conditionals
>> > > around the hooks in kernel_read_file(), and limiting the multiple
>> > > security_kernel_post_read_file() calls to just the compressed case ...
>> > > and honestly in each of the _post_read_file() calls in the compressed
>> > > case, the buffer contents have changed so it somewhat makes sense.
>> >
>> > > Given the code below, IMA could simply ignore the
>> > > READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
>> > > in that case) and focus on the READING_MODULE case as it does today.
>> > > I expect the associated IMA patch would be both trivial and small.
>> > >
>> > > diff --git a/kernel/module/main.c b/kernel/module/main.c
>> > > index c66b26184936..b435c498ec01 100644
>> > > --- a/kernel/module/main.c
>> > > +++ b/kernel/module/main.c
>> > > @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
>> > > ent *u)
>> > >
>> > > static int init_module_from_file(struct file *f, const char __user * uargs, int
>> > > flags)
>> > > {
>> > > +       bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
>> > >        struct load_info info = { };
>> > >        void *buf = NULL;
>> > >        int len;
>> > >
>> > > -       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
>> > > +       len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
>> > > +                              compressed ? READING_MODULE_COMPRESSED : READING_
>> > > MODULE);
>> > >        if (len < 0) {
>> > >                mod_stat_inc(&failed_kreads);
>> > >                return len;
>> > >        }
>> > >
>> > > -       if (flags & MODULE_INIT_COMPRESSED_FILE) {
>> > > +       if (compressed) {
>> > >                int err = module_decompress(&info, buf, len);
>> > >                vfree(buf); /* compressed data is no longer needed */
>> > >                if (err) {
>> > > @@ -3693,6 +3695,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);
>> >
>> > Without changing the enumeration here, IMA would not be able to differentiate
>> > the first call to security_kernel_post_read_file() and this one.  The first call
>> > would result in unnecessary error messages.
>>
>> Given the patch snippet above, in the case where an uncompressed
>> module is passed into init_module_from_file() there would be the
>> following checks, in this order:
>>
>>  * kernel_read_file()
>>  -> security_kernel_read_file(READING_MODULE)
>>  -> security_kernel_post_read_file(READING_MODULE)
>>  * init_module_from_file()
>>  -> NONE
>>
>> ... this should be the same as the current behavior.
>>
>> In the case where a compressed module is passed into
>> init_module_from_file() there would be the following checks, in this
>> order:
>>
>>  * kernel_read_file()
>>  -> security_kernel_read_file(READING_MODULE_COMPRESSED)
>>  -> security_kernel_post_read_file(READING_MODULE_COMPRESSED)
>>  * init_module_from_file()
>>  -> security_kernel_post_read_file(READING_MODULE)
>>
>> ... the two differences being that the hook calls in
>> kernel_read_file() use the READING_MODULE_COMPRESSED id, which seems
>> appropriate as the data passed to the hook is the compressed
>> representation, and the additional _post_read_file() hook call in
>> init_module_from_file() using the READING_MODULE id, as the data
>> passed to the hook is now uncompressed.  Not only should IMA be able
>> to easily differentiate between the two _post_read_file() calls, but
>> it should have access to both the compressed and uncompressed data.
>
>Thanks, Paul.  Yes, a single additional enumeration is enough.

Yeah, thank Paul for elaborating on the solution!

>
>Mimi
>

-- 
Best regards,
Coiby

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

[Mimi Zohar]

On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> [...]
> > 
> > Hi Coiby,
> > 
> > Based on the conversation with Paul, there is no reason to remove the existing
> > security_kernel_post_read_file() call.
> > 
> > The changes are similar to the 2nd link, but a bit different.
> > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > 
> > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > after decompressing the kernel module.  Like a previous version of this patch,
> > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > based on MODULE_INIT_COMPRESSED_FILE.
> > 
> > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> 
> Hi Mimi,
> 
> Thanks for summarizing your conversation with Paul! I can confirm Paul's
> approach works
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> 
> While testing the patch today, I realized there is another
> issue/challenge introduced by in-kernel module decompression. IMA
> appraisal is to verify the digest of compressed kernel module but
> currently the passed buffer is uncompressed module. When IMA uses
> uncompressed module data to calculate the digest, xattr signature
> verification will fail. If we always make IMA read the original kernel
> module data again to calculate the digest, does it look like a
> quick-and-dirty fix? If we can assume people won't load kernel module so
> often, the performance impact is negligible. Otherwise we may have to
> introduce a new LSM hook so IMA can access uncompressed and original
> module data one time.

ima_collect_measurement() stores the file hash info in the iint and uses that
information to verify the signature as stored in the security xattr. 
Decompressing the kernel module shouldn't affect the xattr signature
verification.

The patch with a few minor changes looks good:

- READDING_MODULE_CHECK -> READING_MODULE_CHECK
- Fix the enumeration name in ima_main.c
- scripts/checkpatch.pl code/comment line length has been relaxed to 100 chars,
but the section "Breaking long lines and strings" in
Documentation/process/coding-style.rst still recommends 80 characters.

There are cases where it is necessary to go over the 80 char line limit for
readability, but in general both Roberto and I prefer, as much as possible, to
limit the line length to 80 char.  To detect where/when the line limit is
greater than 80 chars, use the scripts/checkpatch.pl "--max-line-length=80"
option.

After fixing the patch, please post it to linux-integrity mailing list.

-- 
thanks,

Mimi

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

[Mimi Zohar]

On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > [...]
> > > 
> > > Hi Coiby,
> > > 
> > > Based on the conversation with Paul, there is no reason to remove the existing
> > > security_kernel_post_read_file() call.
> > > 
> > > The changes are similar to the 2nd link, but a bit different.
> > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > 
> > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > after decompressing the kernel module.  Like a previous version of this patch,
> > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > based on MODULE_INIT_COMPRESSED_FILE.
> > > 
> > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> > 
> > Hi Mimi,
> > 
> > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > approach works
> > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > 
> > While testing the patch today, I realized there is another
> > issue/challenge introduced by in-kernel module decompression. IMA
> > appraisal is to verify the digest of compressed kernel module but
> > currently the passed buffer is uncompressed module. When IMA uses
> > uncompressed module data to calculate the digest, xattr signature
> > verification will fail. If we always make IMA read the original kernel
> > module data again to calculate the digest, does it look like a
> > quick-and-dirty fix? If we can assume people won't load kernel module so
> > often, the performance impact is negligible. Otherwise we may have to
> > introduce a new LSM hook so IMA can access uncompressed and original
> > module data one time.
> 
> ima_collect_measurement() stores the file hash info in the iint and uses that
> information to verify the signature as stored in the security xattr. 
> Decompressing the kernel module shouldn't affect the xattr signature
> verification.

In the case when the compressed kernel module hasn't previously been measured or
appraised before loading the kernel module, we need to "collect" the file data
hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.

An alternative to your suggestion of re-reading the original kernel module data
to calculate the digest or defining a new hook, would be to define "collect" as
a new "action" and pass the kernel_read_file_id enumeration to
process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
need to be defined.  The new collect "action" should be limited to
func=MODULE_CHECK.

The downside of this alternative is that it requires a new collect rule:
collect func=MODULE_CHECK mask=MAY_READ uid=0
appraise func=MODULE_CHECK appraise_type=imasig|modsig

-- 
thanks,

Mimi

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

[Coiby Xu]

On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
>On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
>> On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
>> > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
>> > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
>> > [...]
>> > >
>> > > Hi Coiby,
>> > >
>> > > Based on the conversation with Paul, there is no reason to remove the existing
>> > > security_kernel_post_read_file() call.
>> > >
>> > > The changes are similar to the 2nd link, but a bit different.
>> > > - Define a single enumeration named READING_MODULE_COMPRESSED.
>> > >
>> > > - In module/main.c add a new security_kernel_post_read_file() call immediately
>> > > after decompressing the kernel module.  Like a previous version of this patch,
>> > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
>> > > based on MODULE_INIT_COMPRESSED_FILE.
>> > >
>> > > - In ima_post_read_file() defer verifying the signature when the enumeration is
>> > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
>> >
>> > Hi Mimi,
>> >
>> > Thanks for summarizing your conversation with Paul! I can confirm Paul's
>> > approach works
>> > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>> >
>> > While testing the patch today, I realized there is another
>> > issue/challenge introduced by in-kernel module decompression. IMA
>> > appraisal is to verify the digest of compressed kernel module but
>> > currently the passed buffer is uncompressed module. When IMA uses
>> > uncompressed module data to calculate the digest, xattr signature
>> > verification will fail. If we always make IMA read the original kernel
>> > module data again to calculate the digest, does it look like a
>> > quick-and-dirty fix? If we can assume people won't load kernel module so
>> > often, the performance impact is negligible. Otherwise we may have to
>> > introduce a new LSM hook so IMA can access uncompressed and original
>> > module data one time.
>>
>> ima_collect_measurement() stores the file hash info in the iint and uses that
>> information to verify the signature as stored in the security xattr.
>> Decompressing the kernel module shouldn't affect the xattr signature
>> verification.
>
>In the case when the compressed kernel module hasn't previously been measured or
>appraised before loading the kernel module, we need to "collect" the file data
>hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
>
>An alternative to your suggestion of re-reading the original kernel module data
>to calculate the digest or defining a new hook, would be to define "collect" as
>a new "action" and pass the kernel_read_file_id enumeration to
>process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
>need to be defined.  The new collect "action" should be limited to
>func=MODULE_CHECK.
>
>The downside of this alternative is that it requires a new collect rule:
>collect func=MODULE_CHECK mask=MAY_READ uid=0
>appraise func=MODULE_CHECK appraise_type=imasig|modsig

Thank for suggesting an alternative! I've implemented the idea in
https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect

Note besides a new collect rule, another change is needed. Currently,
process_measurement only accepts enum ima_hooks thus it can't tell if
it's READING_MODULE_COMPRESSED so to only do collect action. So I
create a fake MODULE_COMPRESSED_CHECK func.

And for the idea of re-reading the original kernel module data, it has
been implemented in 
https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul

Both branches have applied your requested three changes including
respecting the 80 char line limit. Additionally, I made a change to the
IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
enumerate.

After comparing the two implementations, personally I prefer re-reading
the original kernel module data because the change is smaller and it's
more user-friendly. But if there are other reasons I don't know, I'll
post the patches of the new collect action approach to the mailing list.


-- 
Best regards,
Coiby

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

[Mimi Zohar]

On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > > > [...]
> > > > > 
> > > > > Hi Coiby,
> > > > > 
> > > > > Based on the conversation with Paul, there is no reason to remove the existing
> > > > > security_kernel_post_read_file() call.
> > > > > 
> > > > > The changes are similar to the 2nd link, but a bit different.
> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > > > 
> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > > > after decompressing the kernel module.  Like a previous version of this patch,
> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > > > based on MODULE_INIT_COMPRESSED_FILE.
> > > > > 
> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> > > > 
> > > > Hi Mimi,
> > > > 
> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > > > approach works
> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > > > 
> > > > While testing the patch today, I realized there is another
> > > > issue/challenge introduced by in-kernel module decompression. IMA
> > > > appraisal is to verify the digest of compressed kernel module but
> > > > currently the passed buffer is uncompressed module. When IMA uses
> > > > uncompressed module data to calculate the digest, xattr signature
> > > > verification will fail. If we always make IMA read the original kernel
> > > > module data again to calculate the digest, does it look like a
> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
> > > > often, the performance impact is negligible. Otherwise we may have to
> > > > introduce a new LSM hook so IMA can access uncompressed and original
> > > > module data one time.
> > > 
> > > ima_collect_measurement() stores the file hash info in the iint and uses that
> > > information to verify the signature as stored in the security xattr.
> > > Decompressing the kernel module shouldn't affect the xattr signature
> > > verification.
> > 
> > In the case when the compressed kernel module hasn't previously been measured or
> > appraised before loading the kernel module, we need to "collect" the file data
> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
> > 
> > An alternative to your suggestion of re-reading the original kernel module data
> > to calculate the digest or defining a new hook, would be to define "collect" as
> > a new "action" and pass the kernel_read_file_id enumeration to
> > process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
> > need to be defined.  The new collect "action" should be limited to
> > func=MODULE_CHECK.
> > 
> > The downside of this alternative is that it requires a new collect rule:
> > collect func=MODULE_CHECK mask=MAY_READ uid=0
> > appraise func=MODULE_CHECK appraise_type=imasig|modsig

As it turns out, the "collect" rule is unnecessary.  On
READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
file hash.  Extending the IMA measurement list and verifying the signature can
then be differed to READING_MODULE.

> 
> Thank for suggesting an alternative! I've implemented the idea in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
> 
> Note besides a new collect rule, another change is needed. Currently,
> process_measurement only accepts enum ima_hooks thus it can't tell if
> it's READING_MODULE_COMPRESSED so to only do collect action. So I
> create a fake MODULE_COMPRESSED_CHECK func.

Correct, either extending process_measurement() with the read_idmap enum or
defining the fake hook would work.

> 
> And for the idea of re-reading the original kernel module data, it has
> been implemented in 
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> 
> Both branches have applied your requested three changes including
> respecting the 80 char line limit. Additionally, I made a change to the
> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
> enumerate.
> 
> After comparing the two implementations, personally I prefer re-reading
> the original kernel module data because the change is smaller and it's
> more user-friendly. But if there are other reasons I don't know, I'll
> post the patches of the new collect action approach to the mailing list.

The "re-reading" option fails some of the tests.  As the "collect" rule isn't
needed, let's stick with the first option.

-- 

thanks,

Mimi

[PATCH v3] ima: Access decompressed kernel module to verify appended signature

[Coiby Xu]

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 kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can know only to collect original module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.

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>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kernel_read_file.h    |  1 +
 kernel/module/main.c                | 17 ++++++++++++++---
 security/integrity/ima/ima_main.c   | 24 ++++++++++++++++--------
 security/integrity/ima/ima_policy.c |  3 ++-
 security/ipe/hooks.c                |  1 +
 security/selinux/hooks.c            |  5 +++--
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(MODULE_COMPRESSED, kernel-module-compressed) \
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
 
 static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
 {
+	bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
 	struct load_info info = { };
 	void *buf = NULL;
 	int len;
+	int err;
 
-	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+			       compressed ? READING_MODULE_COMPRESSED :
+					    READING_MODULE);
 	if (len < 0) {
 		mod_stat_inc(&failed_kreads);
 		return len;
 	}
 
-	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+	if (compressed) {
+		err = module_decompress(&info, buf, len);
 		vfree(buf); /* compressed data is no longer needed */
 		if (err) {
 			mod_stat_inc(&failed_decompress);
 			mod_stat_add_long(len, &invalid_decompress_bytes);
 			return err;
 		}
+		err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+						     READING_MODULE);
+		if (err) {
+			mod_stat_inc(&failed_kreads);
+			free_copy(&info, flags);
+			return err;
+		}
 	} else {
 		info.hdr = buf;
 		info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cdd225f65a62..49f8b2b1a9af 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
 
 static int process_measurement(struct file *file, const struct cred *cred,
 			       struct lsm_prop *prop, char *buf, loff_t size,
-			       int mask, enum ima_hooks func)
+			       int mask, enum ima_hooks func,
+			       enum kernel_read_file_id read_id)
 {
 	struct inode *real_inode, *inode = file_inode(file);
 	struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
+	/* Defer measuring/appraising kernel modules to READING_MODULE */
+	if (read_id == READING_MODULE_COMPRESSED) {
+		must_appraise = 0;
+		goto out_locked;
+	}
+
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
 
 	if (reqprot & PROT_EXEC) {
 		ret = process_measurement(file, current_cred(), &prop, NULL,
-					  0, MAY_EXEC, MMAP_CHECK_REQPROT);
+					  0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
 		if (ret)
 			return ret;
 	}
 
 	if (prot & PROT_EXEC)
 		return process_measurement(file, current_cred(), &prop, NULL,
-					   0, MAY_EXEC, MMAP_CHECK);
+					   0, MAY_EXEC, MMAP_CHECK, 0);
 
 	return 0;
 }
@@ -578,13 +585,13 @@ static int ima_bprm_check(struct linux_binprm *bprm)
 
 	security_current_getlsmprop_subj(&prop);
 	ret = process_measurement(bprm->file, current_cred(),
-				  &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+				  &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
 	if (ret)
 		return ret;
 
 	security_cred_getlsmprop(bprm->cred, &prop);
 	return process_measurement(bprm->file, bprm->cred, &prop, NULL, 0,
-				   MAY_EXEC, CREDS_CHECK);
+				   MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -632,7 +639,7 @@ static int ima_file_check(struct file *file, int mask)
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK);
+					   MAY_APPEND), FILE_CHECK, 0);
 }
 
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -851,12 +858,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
-				   MAY_READ, func);
+				   MAY_READ, func, 0);
 }
 
 const int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
+	[READING_MODULE_COMPRESSED] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
 	[READING_POLICY] = POLICY_CHECK
@@ -894,7 +902,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, buf, size,
-				   MAY_READ, func);
+				   MAY_READ, func, read_id);
 }
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..ae520e6bb1cf 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -241,7 +241,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+		  IMA_CHECK_BLACKLIST},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
 		op = IPE_OP_FIRMWARE;
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		op = IPE_OP_KERNEL_MODULE;
 		break;
 	case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
 			 "New kernel_read_file_id introduced; update SELinux!");
 
 	switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
 		rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
 		break;
 	case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
 			 "New kernel_load_data_id introduced; update SELinux!");
 
 	switch (id) {

base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
-- 
2.51.1

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

[Coiby Xu]

On Tue, Nov 18, 2025 at 07:19:50AM -0500, Mimi Zohar wrote:
>On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
>> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
>> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
>> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
>> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
>> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
>> > > > [...]
>> > > > >
>> > > > > Hi Coiby,
>> > > > >
>> > > > > Based on the conversation with Paul, there is no reason to remove the existing
>> > > > > security_kernel_post_read_file() call.
>> > > > >
>> > > > > The changes are similar to the 2nd link, but a bit different.
>> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
>> > > > >
>> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
>> > > > > after decompressing the kernel module.  Like a previous version of this patch,
>> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
>> > > > > based on MODULE_INIT_COMPRESSED_FILE.
>> > > > >
>> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
>> > > > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
>> > > >
>> > > > Hi Mimi,
>> > > >
>> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
>> > > > approach works
>> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>> > > >
>> > > > While testing the patch today, I realized there is another
>> > > > issue/challenge introduced by in-kernel module decompression. IMA
>> > > > appraisal is to verify the digest of compressed kernel module but
>> > > > currently the passed buffer is uncompressed module. When IMA uses
>> > > > uncompressed module data to calculate the digest, xattr signature
>> > > > verification will fail. If we always make IMA read the original kernel
>> > > > module data again to calculate the digest, does it look like a
>> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
>> > > > often, the performance impact is negligible. Otherwise we may have to
>> > > > introduce a new LSM hook so IMA can access uncompressed and original
>> > > > module data one time.
>> > >
>> > > ima_collect_measurement() stores the file hash info in the iint and uses that
>> > > information to verify the signature as stored in the security xattr.
>> > > Decompressing the kernel module shouldn't affect the xattr signature
>> > > verification.
>> >
>> > In the case when the compressed kernel module hasn't previously been measured or
>> > appraised before loading the kernel module, we need to "collect" the file data
>> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
>> >
>> > An alternative to your suggestion of re-reading the original kernel module data
>> > to calculate the digest or defining a new hook, would be to define "collect" as
>> > a new "action" and pass the kernel_read_file_id enumeration to
>> > process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
>> > need to be defined.  The new collect "action" should be limited to
>> > func=MODULE_CHECK.
>> >
>> > The downside of this alternative is that it requires a new collect rule:
>> > collect func=MODULE_CHECK mask=MAY_READ uid=0
>> > appraise func=MODULE_CHECK appraise_type=imasig|modsig
>
>As it turns out, the "collect" rule is unnecessary.  On
>READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
>file hash.  Extending the IMA measurement list and verifying the signature can
>then be differed to READING_MODULE.
>
>>
>> Thank for suggesting an alternative! I've implemented the idea in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
>>
>> Note besides a new collect rule, another change is needed. Currently,
>> process_measurement only accepts enum ima_hooks thus it can't tell if
>> it's READING_MODULE_COMPRESSED so to only do collect action. So I
>> create a fake MODULE_COMPRESSED_CHECK func.
>
>Correct, either extending process_measurement() with the read_idmap enum or
>defining the fake hook would work.
>
>>
>> And for the idea of re-reading the original kernel module data, it has
>> been implemented in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>>
>> Both branches have applied your requested three changes including
>> respecting the 80 char line limit. Additionally, I made a change to the
>> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
>> enumerate.
>>
>> After comparing the two implementations, personally I prefer re-reading
>> the original kernel module data because the change is smaller and it's
>> more user-friendly. But if there are other reasons I don't know, I'll
>> post the patches of the new collect action approach to the mailing list.
>
>The "re-reading" option fails some of the tests.  As the "collect" rule isn't
>needed, let's stick with the first option.

Thanks for evaluating the two options! Yeah, without the "collect" rule,
the 1st option is much better as it doesn't have the issue of re-reading
the module.

-- 
Best regards,
Coiby

Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature

[Mimi Zohar]

Hi Coiby,

On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu 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 kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
> IMA can know only to collect original module data hash on
> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
> READING_MODULE when the module has been decompressed.

This paragraph is a bit awkward.  Perhaps something like:

-> so IMA can calculate the compressed kernel module data hash and defer
measuring/appraising ...

> 
> 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>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>

Thanks, Coiby!

The patch applies cleanly to linus' tree, but needs to be applied to next-
integrity.  Please re-base.

-- 

thanks,

Mimi

[PATCH v4] ima: Access decompressed kernel module to verify appended signature

[Coiby Xu]

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 kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can calculate the compressed kernel module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.

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>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kernel_read_file.h    |  1 +
 kernel/module/main.c                | 17 ++++++++++++++---
 security/integrity/ima/ima_main.c   | 24 ++++++++++++++++--------
 security/integrity/ima/ima_policy.c |  3 ++-
 security/ipe/hooks.c                |  1 +
 security/selinux/hooks.c            |  5 +++--
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(MODULE_COMPRESSED, kernel-module-compressed) \
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
 
 static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
 {
+	bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
 	struct load_info info = { };
 	void *buf = NULL;
 	int len;
+	int err;
 
-	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+			       compressed ? READING_MODULE_COMPRESSED :
+					    READING_MODULE);
 	if (len < 0) {
 		mod_stat_inc(&failed_kreads);
 		return len;
 	}
 
-	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+	if (compressed) {
+		err = module_decompress(&info, buf, len);
 		vfree(buf); /* compressed data is no longer needed */
 		if (err) {
 			mod_stat_inc(&failed_decompress);
 			mod_stat_add_long(len, &invalid_decompress_bytes);
 			return err;
 		}
+		err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+						     READING_MODULE);
+		if (err) {
+			mod_stat_inc(&failed_kreads);
+			free_copy(&info, flags);
+			return err;
+		}
 	} else {
 		info.hdr = buf;
 		info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ebaebccfbe9a..edd0fd3d77a0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
 
 static int process_measurement(struct file *file, const struct cred *cred,
 			       struct lsm_prop *prop, char *buf, loff_t size,
-			       int mask, enum ima_hooks func)
+			       int mask, enum ima_hooks func,
+			       enum kernel_read_file_id read_id)
 {
 	struct inode *real_inode, *inode = file_inode(file);
 	struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
+	/* Defer measuring/appraising kernel modules to READING_MODULE */
+	if (read_id == READING_MODULE_COMPRESSED) {
+		must_appraise = 0;
+		goto out_locked;
+	}
+
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
 
 	if (reqprot & PROT_EXEC) {
 		ret = process_measurement(file, current_cred(), &prop, NULL,
-					  0, MAY_EXEC, MMAP_CHECK_REQPROT);
+					  0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
 		if (ret)
 			return ret;
 	}
 
 	if (prot & PROT_EXEC)
 		return process_measurement(file, current_cred(), &prop, NULL,
-					   0, MAY_EXEC, MMAP_CHECK);
+					   0, MAY_EXEC, MMAP_CHECK, 0);
 
 	return 0;
 }
@@ -577,7 +584,7 @@ static int ima_bprm_check(struct linux_binprm *bprm)
 
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(bprm->file, current_cred(),
-				   &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+				   &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
 }
 
 /**
@@ -607,7 +614,7 @@ static int ima_creds_check(struct linux_binprm *bprm, const struct file *file)
 
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement((struct file *)file, bprm->cred, &prop, NULL,
-				   0, MAY_EXEC, CREDS_CHECK);
+				   0, MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -655,7 +662,7 @@ static int ima_file_check(struct file *file, int mask)
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK);
+					   MAY_APPEND), FILE_CHECK, 0);
 }
 
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -874,12 +881,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
-				   MAY_READ, func);
+				   MAY_READ, func, 0);
 }
 
 const int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
+	[READING_MODULE_COMPRESSED] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
 	[READING_POLICY] = POLICY_CHECK
@@ -917,7 +925,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, buf, size,
-				   MAY_READ, func);
+				   MAY_READ, func, read_id);
 }
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 164d62832f8e..7468afaab686 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -244,7 +244,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+		  IMA_CHECK_BLACKLIST},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
 		op = IPE_OP_FIRMWARE;
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		op = IPE_OP_KERNEL_MODULE;
 		break;
 	case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
 			 "New kernel_read_file_id introduced; update SELinux!");
 
 	switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
 		rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
 		break;
 	case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
 			 "New kernel_load_data_id introduced; update SELinux!");
 
 	switch (id) {

base-commit: 43369273518f57b7d56c1cf12d636a809b7bd81b
-- 
2.51.1

Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature

[Coiby Xu]

On Wed, Nov 19, 2025 at 08:29:22AM -0500, Mimi Zohar wrote:
>Hi Coiby,

Hi Mimi,

>
>On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu 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 kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
>> IMA can know only to collect original module data hash on
>> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
>> READING_MODULE when the module has been decompressed.
>
>This paragraph is a bit awkward.  Perhaps something like:
>
>-> so IMA can calculate the compressed kernel module data hash and defer
>measuring/appraising ...
>
>>
>> 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>
>> Suggested-by: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Thanks, Coiby!
>
>The patch applies cleanly to linus' tree, but needs to be applied to next-
>integrity.  Please re-base.

I've sent v4 which has been rebased onto next tree with improved
wording as suggested.

>
>-- 
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby

Re: [PATCH v4] ima: Access decompressed kernel module to verify appended signature

[Mimi Zohar]

On Wed, 2025-11-19 at 22:03 +0800, Coiby Xu 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 kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
> IMA can calculate the compressed kernel module data hash on
> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
> READING_MODULE when the module has been decompressed.
> 
> 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>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>

Thanks, Coiby!  The patch is now queued in next-integrity.

-- 
Mimi