[PATCH] ima: Fall back to default kernel module signature verification

[PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

Currently, for any IMA policy that requires appraisal for kernel modules
e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
booting will fail because IMA will reject a kernel module which will
be decompressed in the kernel space and then have its signature
verified.

This happens because when in-kernel module decompression
(CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
syscall instead of init_module to load a module. And IMA mandates IMA
xattr verification for finit_module unless appraise_type=imasig|modsig
is specified in the rule.  However currently initramfs doesn't support
xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
doesn't work either because IMA will treat to-be-decompressed kernel
module as not having module signature as it can't decompress kernel
module to check if signature exists.

So fall back to default kernel module signature verification when we have
no way to verify IMA xattr.

Reported-by: Karel Srot <ksrot@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
Another approach will be to make IMA decompress the kernel module to
check the signature. This requires refactoring kernel module code to
make the in-kernel module decompressing feature modular and seemingly
more efforts are needed. A second disadvantage is it feels
counter-intuitive to verify the same kernel module signature twice. And
we still need to make ima_policy=secure_boot allow verifying appended
module signature.

Anyways, I'm open to suggestions and can try the latter approach if
there are some benefits I'm not aware of or a better approach.

 security/integrity/ima/ima_appraise.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f435eff4667f..fcc75dd1486f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -502,9 +502,10 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len;
 	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
+	bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
 
-	/* If not appraising a modsig, we need an xattr. */
-	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
+	/* If not appraising a modsig or using default module verification, we need an xattr. */
+	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
 		return INTEGRITY_UNKNOWN;
 
 	/*
@@ -517,8 +518,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	if (is_bprm_creds_for_exec(func, file))
 		audit_msgno = AUDIT_INTEGRITY_USERSPACE;
 
-	/* If reading the xattr failed and there's no modsig, error out. */
-	if (rc <= 0 && !try_modsig) {
+	/* If reading the xattr failed and there's no modsig or module verification, error out. */
+	if (rc <= 0 && !try_modsig && !enforce_module_sig) {
 		if (rc && rc != -ENODATA)
 			goto out;
 
@@ -549,8 +550,8 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	case INTEGRITY_UNKNOWN:
 		break;
 	case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
-		/* It's fine not to have xattrs when using a modsig. */
-		if (try_modsig)
+		/* Fine to not have xattrs when using a modsig or default module verification. */
+		if (try_modsig || enforce_module_sig)
 			break;
 		fallthrough;
 	case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
@@ -580,6 +581,18 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	     rc == -ENOKEY))
 		rc = modsig_verify(func, modsig, &status, &cause);
 
+	/* Fall back to default kernel module signature verification */
+	if (rc && enforce_module_sig) {
+		rc = 0;
+		set_module_sig_enforced();
+		/* CONFIG_MODULE_SIG may be disabled */
+		if (is_module_sig_enforced()) {
+			rc = 0;
+			status = INTEGRITY_PASS;
+			pr_debug("Fall back to default kernel module verification for %s\n", filename);
+		}
+	}
+
 out:
 	/*
 	 * File signatures on some filesystems can not be properly verified.

base-commit: cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
-- 
2.51.0

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> Currently, for any IMA policy that requires appraisal for kernel modules
> e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> booting will fail because IMA will reject a kernel module which will
> be decompressed in the kernel space and then have its signature
> verified.
> 
> This happens because when in-kernel module decompression
> (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> syscall instead of init_module to load a module. And IMA mandates IMA
> xattr verification for finit_module unless appraise_type=imasig|modsig
> is specified in the rule.  However currently initramfs doesn't support
> xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> doesn't work either because IMA will treat to-be-decompressed kernel
> module as not having module signature as it can't decompress kernel
> module to check if signature exists.
> 
> So fall back to default kernel module signature verification when we have
> no way to verify IMA xattr.
> 
> Reported-by: Karel Srot <ksrot@redhat.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
> Another approach will be to make IMA decompress the kernel module to
> check the signature. This requires refactoring kernel module code to
> make the in-kernel module decompressing feature modular and seemingly
> more efforts are needed. A second disadvantage is it feels
> counter-intuitive to verify the same kernel module signature twice. And
> we still need to make ima_policy=secure_boot allow verifying appended
> module signature.
> 
> Anyways, I'm open to suggestions and can try the latter approach if
> there are some benefits I'm not aware of or a better approach.

Coiby, there are multiple issues being discussed here.  Before deciding on an
appropriate solution, let's frame the issues(s) properly.

1. The finit_module syscall eventually calls init_module_from_file() to read the
module into memory and then decompress it.  The problem is that the kernel
module signature verification occurs during the kernel_read_file(), before the
kernel module is decompressed.  Thus, the appended kernel module signature
cannot be verified.

2. CPIO doesn't have xattr support. There were multiple attempts at including
xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
security.ima were available in the initramfs, then finit_module() could verify
them, as opposed to the appended kernel module signature.

3. The issues described above are generic, not limited to Power.  When
CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
include an "appraise func=MODULE_CHECK".

4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
policy, specified on the boot command line as "ima_policy=secure_boot", always
enforces the IMA signature stored in security.ima.

Partial solutions without kernel changes:
- Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
- Disable kernel module compression.

Complete solution:
- Pick up and upstream Roberto Sassu's last version of initramfs support [1].
- Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
enumeration is READING_MODULE and the kernel module is compressed.  The change
might be limited to ima_post_read_file().

thanks,

Mimi

[1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
> On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> > Currently, for any IMA policy that requires appraisal for kernel modules
> > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> > booting will fail because IMA will reject a kernel module which will
> > be decompressed in the kernel space and then have its signature
> > verified.
> > 
> > This happens because when in-kernel module decompression
> > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> > syscall instead of init_module to load a module. And IMA mandates IMA
> > xattr verification for finit_module unless appraise_type=imasig|modsig
> > is specified in the rule.  However currently initramfs doesn't support
> > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> > doesn't work either because IMA will treat to-be-decompressed kernel
> > module as not having module signature as it can't decompress kernel
> > module to check if signature exists.
> > 
> > So fall back to default kernel module signature verification when we have
> > no way to verify IMA xattr.
> > 
> > Reported-by: Karel Srot <ksrot@redhat.com>
> > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > ---
> > Another approach will be to make IMA decompress the kernel module to
> > check the signature. This requires refactoring kernel module code to
> > make the in-kernel module decompressing feature modular and seemingly
> > more efforts are needed. A second disadvantage is it feels
> > counter-intuitive to verify the same kernel module signature twice. And
> > we still need to make ima_policy=secure_boot allow verifying appended
> > module signature.
> > 
> > Anyways, I'm open to suggestions and can try the latter approach if
> > there are some benefits I'm not aware of or a better approach.
> 
> Coiby, there are multiple issues being discussed here.  Before deciding on an
> appropriate solution, let's frame the issues(s) properly.
> 
> 1. The finit_module syscall eventually calls init_module_from_file() to read the
> module into memory and then decompress it.  The problem is that the kernel
> module signature verification occurs during the kernel_read_file(), before the
> kernel module is decompressed.  Thus, the appended kernel module signature
> cannot be verified.
> 
> 2. CPIO doesn't have xattr support. There were multiple attempts at including
> xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
> security.ima were available in the initramfs, then finit_module() could verify
> them, as opposed to the appended kernel module signature.
> 
> 3. The issues described above are generic, not limited to Power.  When
> CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
> include an "appraise func=MODULE_CHECK".
> 
> 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
> policy, specified on the boot command line as "ima_policy=secure_boot", always
> enforces the IMA signature stored in security.ima.
> 
> Partial solutions without kernel changes:
> - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
> - Disable kernel module compression.
> 
> Complete solution:
> - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
> - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
> enumeration is READING_MODULE and the kernel module is compressed.  The change
> might be limited to ima_post_read_file().

or perhaps not totally.

init_module_from_file() doesn't pass the flags variable to kernel_read_file(). 
You might want to consider defining a new kernel_read_file_id enumeration named
READING_COMPRESSED_MODULE.

Mimi

> 
> [1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
> https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/
> 
> 

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
>On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
>> On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
>> > Currently, for any IMA policy that requires appraisal for kernel modules
>> > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
>> > booting will fail because IMA will reject a kernel module which will
>> > be decompressed in the kernel space and then have its signature
>> > verified.
>> >
>> > This happens because when in-kernel module decompression
>> > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
>> > syscall instead of init_module to load a module. And IMA mandates IMA
>> > xattr verification for finit_module unless appraise_type=imasig|modsig
>> > is specified in the rule.  However currently initramfs doesn't support
>> > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
>> > doesn't work either because IMA will treat to-be-decompressed kernel
>> > module as not having module signature as it can't decompress kernel
>> > module to check if signature exists.
>> >
>> > So fall back to default kernel module signature verification when we have
>> > no way to verify IMA xattr.
>> >
>> > Reported-by: Karel Srot <ksrot@redhat.com>
>> > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > ---
>> > Another approach will be to make IMA decompress the kernel module to
>> > check the signature. This requires refactoring kernel module code to
>> > make the in-kernel module decompressing feature modular and seemingly
>> > more efforts are needed. A second disadvantage is it feels
>> > counter-intuitive to verify the same kernel module signature twice. And
>> > we still need to make ima_policy=secure_boot allow verifying appended
>> > module signature.
>> >
>> > Anyways, I'm open to suggestions and can try the latter approach if
>> > there are some benefits I'm not aware of or a better approach.
>>
>> Coiby, there are multiple issues being discussed here.  Before deciding on an
>> appropriate solution, let's frame the issues(s) properly.

Hi Mimi,

Thanks for listing and framing the issues! Sorry, it took me a while to
go through your feedback as I also had a 8-day holiday.

>>
>> 1. The finit_module syscall eventually calls init_module_from_file() to read the
>> module into memory and then decompress it.  The problem is that the kernel
>> module signature verification occurs during the kernel_read_file(), before the
>> kernel module is decompressed.  Thus, the appended kernel module signature
>> cannot be verified.

Since IMA only accesses a kernel module as a fd or struct file*, even if
IMA hook is triggerd after kernel module is decompressed, IMA still
doesn't know the default verification has passed or can't access the
decompressed kernel buffer [2] to do the verification by itself.

[2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689

>>
>> 2. CPIO doesn't have xattr support. There were multiple attempts at including
>> xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
>> security.ima were available in the initramfs, then finit_module() could verify
>> them, as opposed to the appended kernel module signature.

Thanks you for pointing me to the work [1]. I'll take a more careful
look at [1]. I think CPIO supporting xattr can be a long-term solution
and also close a important security gap.

>>
>> 3. The issues described above are generic, not limited to Power.  When
>> CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
>> include an "appraise func=MODULE_CHECK".

Yes, the issue is not limited to Power. And thanks for correcting me
that Power arch specific IMA policy rules don't have this problem! Sorry
I misread the code.

>>
>> 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
>> policy, specified on the boot command line as "ima_policy=secure_boot", always
>> enforces the IMA signature stored in security.ima.
>>
>> Partial solutions without kernel changes:
>> - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
>> - Disable kernel module compression.
>>
>> Complete solution:
>> - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
>> - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
>> enumeration is READING_MODULE and the kernel module is compressed.  The change
>> might be limited to ima_post_read_file().
>
>or perhaps not totally.
>
>init_module_from_file() doesn't pass the flags variable to kernel_read_file().
>You might want to consider defining a new kernel_read_file_id enumeration named
>READING_COMPRESSED_MODULE.

Thanks for suggesting the solutions! I like the solution of CPIO
supporting xattr but it seems it won't land in upstream soon. So I
prefer the last approach. I've implemented one [3] by defining a new
kernel_read_file_id enumeration, would you like me to post the patches
to the mailing list directly?

[3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima

>
>Mimi
>
>>
>> [1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
>> https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/
>>
>>
>
>

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Thu, 2025-10-16 at 11:46 +0800, Coiby Xu wrote:
> On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
> > On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
> > > On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> > > > Currently, for any IMA policy that requires appraisal for kernel modules
> > > > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> > > > booting will fail because IMA will reject a kernel module which will
> > > > be decompressed in the kernel space and then have its signature
> > > > verified.
> > > > 
> > > > This happens because when in-kernel module decompression
> > > > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> > > > syscall instead of init_module to load a module. And IMA mandates IMA
> > > > xattr verification for finit_module unless appraise_type=imasig|modsig
> > > > is specified in the rule.  However currently initramfs doesn't support
> > > > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> > > > doesn't work either because IMA will treat to-be-decompressed kernel
> > > > module as not having module signature as it can't decompress kernel
> > > > module to check if signature exists.
> > > > 
> > > > So fall back to default kernel module signature verification when we have
> > > > no way to verify IMA xattr.
> > > > 
> > > > Reported-by: Karel Srot <ksrot@redhat.com>
> > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > ---
> > > > Another approach will be to make IMA decompress the kernel module to
> > > > check the signature. This requires refactoring kernel module code to
> > > > make the in-kernel module decompressing feature modular and seemingly
> > > > more efforts are needed. A second disadvantage is it feels
> > > > counter-intuitive to verify the same kernel module signature twice. And
> > > > we still need to make ima_policy=secure_boot allow verifying appended
> > > > module signature.
> > > > 
> > > > Anyways, I'm open to suggestions and can try the latter approach if
> > > > there are some benefits I'm not aware of or a better approach.
> > > 
> > > Coiby, there are multiple issues being discussed here.  Before deciding on an
> > > appropriate solution, let's frame the issues(s) properly.
> 
> Hi Mimi,
> 
> Thanks for listing and framing the issues! Sorry, it took me a while to
> go through your feedback as I also had a 8-day holiday.
> 
> > > 
> > > 1. The finit_module syscall eventually calls init_module_from_file() to read the
> > > module into memory and then decompress it.  The problem is that the kernel
> > > module signature verification occurs during the kernel_read_file(), before the
> > > kernel module is decompressed.  Thus, the appended kernel module signature
> > > cannot be verified.
> 
> Since IMA only accesses a kernel module as a fd or struct file*, even if
> IMA hook is triggerd after kernel module is decompressed, IMA still
> doesn't know the default verification has passed or can't access the
> decompressed kernel buffer [2] to do the verification by itself.
> 
> [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
> 
> > > 
> > > 2. CPIO doesn't have xattr support. There were multiple attempts at including
> > > xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
> > > security.ima were available in the initramfs, then finit_module() could verify
> > > them, as opposed to the appended kernel module signature.
> 
> Thanks you for pointing me to the work [1]. I'll take a more careful
> look at [1]. I think CPIO supporting xattr can be a long-term solution
> and also close a important security gap.
> 
> > > 
> > > 3. The issues described above are generic, not limited to Power.  When
> > > CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
> > > include an "appraise func=MODULE_CHECK".
> 
> Yes, the issue is not limited to Power. And thanks for correcting me
> that Power arch specific IMA policy rules don't have this problem! Sorry
> I misread the code.
> 
> > > 
> > > 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
> > > policy, specified on the boot command line as "ima_policy=secure_boot", always
> > > enforces the IMA signature stored in security.ima.
> > > 
> > > Partial solutions without kernel changes:
> > > - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
> > > - Disable kernel module compression.
> > > 
> > > Complete solution:
> > > - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
> > > - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
> > > enumeration is READING_MODULE and the kernel module is compressed.  The change
> > > might be limited to ima_post_read_file().
> > 
> > or perhaps not totally.
> > 
> > init_module_from_file() doesn't pass the flags variable to kernel_read_file().
> > You might want to consider defining a new kernel_read_file_id enumeration named
> > READING_COMPRESSED_MODULE.
> 
> Thanks for suggesting the solutions! I like the solution of CPIO
> supporting xattr but it seems it won't land in upstream soon. So I
> prefer the last approach. I've implemented one [3] by defining a new
> kernel_read_file_id enumeration, would you like me to post the patches
> to the mailing list directly?
> 
> [3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima

A few thoughts, before you post the patch.

1. In the general case, the kernel module could be compressed, but without an
appended signature.  The new code should only defer appended signature
verification, if there isn't an xattr or appended signature.

2. Instead of defining an additional process_measurement() argument to identify
compressed kernel modules, to simplify the code it might be possible to define a
new "func" named COMPRESSED_MODULE_CHECK.

 +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK

3.  The patch title "ima: Use default kernel module signature verification for
compressed ones" is a bit off.  It should be something along the lines of "ima:
defer compressed kernel module appended signature verification".

4. Simplify the patch description.

> > 
Mimi

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Thu, Oct 16, 2025 at 10:31:35PM -0400, Mimi Zohar wrote:
>On Thu, 2025-10-16 at 11:46 +0800, Coiby Xu wrote:
>> On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
>> > On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
>> > > On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
>> > > > Currently, for any IMA policy that requires appraisal for kernel modules
>> > > > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
>> > > > booting will fail because IMA will reject a kernel module which will
>> > > > be decompressed in the kernel space and then have its signature
>> > > > verified.
>> > > >
>> > > > This happens because when in-kernel module decompression
>> > > > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
>> > > > syscall instead of init_module to load a module. And IMA mandates IMA
>> > > > xattr verification for finit_module unless appraise_type=imasig|modsig
>> > > > is specified in the rule.  However currently initramfs doesn't support
>> > > > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
>> > > > doesn't work either because IMA will treat to-be-decompressed kernel
>> > > > module as not having module signature as it can't decompress kernel
>> > > > module to check if signature exists.
>> > > >
>> > > > So fall back to default kernel module signature verification when we have
>> > > > no way to verify IMA xattr.
>> > > >
>> > > > Reported-by: Karel Srot <ksrot@redhat.com>
>> > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > > ---
>> > > > Another approach will be to make IMA decompress the kernel module to
>> > > > check the signature. This requires refactoring kernel module code to
>> > > > make the in-kernel module decompressing feature modular and seemingly
>> > > > more efforts are needed. A second disadvantage is it feels
>> > > > counter-intuitive to verify the same kernel module signature twice. And
>> > > > we still need to make ima_policy=secure_boot allow verifying appended
>> > > > module signature.
>> > > >
>> > > > Anyways, I'm open to suggestions and can try the latter approach if
>> > > > there are some benefits I'm not aware of or a better approach.
>> > >
>> > > Coiby, there are multiple issues being discussed here.  Before deciding on an
>> > > appropriate solution, let's frame the issues(s) properly.
>>
>> Hi Mimi,
>>
>> Thanks for listing and framing the issues! Sorry, it took me a while to
>> go through your feedback as I also had a 8-day holiday.
>>
>> > >
>> > > 1. The finit_module syscall eventually calls init_module_from_file() to read the
>> > > module into memory and then decompress it.  The problem is that the kernel
>> > > module signature verification occurs during the kernel_read_file(), before the
>> > > kernel module is decompressed.  Thus, the appended kernel module signature
>> > > cannot be verified.
>>
>> Since IMA only accesses a kernel module as a fd or struct file*, even if
>> IMA hook is triggerd after kernel module is decompressed, IMA still
>> doesn't know the default verification has passed or can't access the
>> decompressed kernel buffer [2] to do the verification by itself.
>>
>> [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
>>
>> > >
>> > > 2. CPIO doesn't have xattr support. There were multiple attempts at including
>> > > xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
>> > > security.ima were available in the initramfs, then finit_module() could verify
>> > > them, as opposed to the appended kernel module signature.
>>
>> Thanks you for pointing me to the work [1]. I'll take a more careful
>> look at [1]. I think CPIO supporting xattr can be a long-term solution
>> and also close a important security gap.
>>
>> > >
>> > > 3. The issues described above are generic, not limited to Power.  When
>> > > CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
>> > > include an "appraise func=MODULE_CHECK".
>>
>> Yes, the issue is not limited to Power. And thanks for correcting me
>> that Power arch specific IMA policy rules don't have this problem! Sorry
>> I misread the code.
>>
>> > >
>> > > 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
>> > > policy, specified on the boot command line as "ima_policy=secure_boot", always
>> > > enforces the IMA signature stored in security.ima.
>> > >
>> > > Partial solutions without kernel changes:
>> > > - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
>> > > - Disable kernel module compression.
>> > >
>> > > Complete solution:
>> > > - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
>> > > - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
>> > > enumeration is READING_MODULE and the kernel module is compressed.  The change
>> > > might be limited to ima_post_read_file().
>> >
>> > or perhaps not totally.
>> >
>> > init_module_from_file() doesn't pass the flags variable to kernel_read_file().
>> > You might want to consider defining a new kernel_read_file_id enumeration named
>> > READING_COMPRESSED_MODULE.
>>
>> Thanks for suggesting the solutions! I like the solution of CPIO
>> supporting xattr but it seems it won't land in upstream soon. So I
>> prefer the last approach. I've implemented one [3] by defining a new
>> kernel_read_file_id enumeration, would you like me to post the patches
>> to the mailing list directly?
>>
>> [3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima
>
>A few thoughts, before you post the patch.

Thank you for sharing your thoughts!

>
>1. In the general case, the kernel module could be compressed, but without an
>appended signature.  The new code should only defer appended signature
>verification, if there isn't an xattr or appended signature.

I'll add these two condition checks, thanks!

>
>2. Instead of defining an additional process_measurement() argument to identify
>compressed kernel modules, to simplify the code it might be possible to define a
>new "func" named COMPRESSED_MODULE_CHECK.
>
> +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK

I also thought about this approach. But IMA rule maps kernel module
loading to MODULE_CHECK. If we define a new rule and ask users to use
this new rule, ima_policy=secure_boot still won't work.

>
>3.  The patch title "ima: Use default kernel module signature verification for
>compressed ones" is a bit off.  It should be something along the lines of "ima:
>defer compressed kernel module appended signature verification".

>
>4. Simplify the patch description.

I'll rephrase the title and try simplifying it. Thanks!

>
>> >
>Mimi
>

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Fri, 2025-10-17 at 11:19 +0800, Coiby Xu wrote:
> On Thu, Oct 16, 2025 at 10:31:35PM -0400, Mimi Zohar wrote:
> > On Thu, 2025-10-16 at 11:46 +0800, Coiby Xu wrote:
> > > On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
> > > > On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
> > > > > On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
> > > > > > Currently, for any IMA policy that requires appraisal for kernel modules
> > > > > > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
> > > > > > booting will fail because IMA will reject a kernel module which will
> > > > > > be decompressed in the kernel space and then have its signature
> > > > > > verified.
> > > > > > 
> > > > > > This happens because when in-kernel module decompression
> > > > > > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
> > > > > > syscall instead of init_module to load a module. And IMA mandates IMA
> > > > > > xattr verification for finit_module unless appraise_type=imasig|modsig
> > > > > > is specified in the rule.  However currently initramfs doesn't support
> > > > > > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
> > > > > > doesn't work either because IMA will treat to-be-decompressed kernel
> > > > > > module as not having module signature as it can't decompress kernel
> > > > > > module to check if signature exists.
> > > > > > 
> > > > > > So fall back to default kernel module signature verification when we have
> > > > > > no way to verify IMA xattr.
> > > > > > 
> > > > > > Reported-by: Karel Srot <ksrot@redhat.com>
> > > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > > > ---
> > > > > > Another approach will be to make IMA decompress the kernel module to
> > > > > > check the signature. This requires refactoring kernel module code to
> > > > > > make the in-kernel module decompressing feature modular and seemingly
> > > > > > more efforts are needed. A second disadvantage is it feels
> > > > > > counter-intuitive to verify the same kernel module signature twice. And
> > > > > > we still need to make ima_policy=secure_boot allow verifying appended
> > > > > > module signature.
> > > > > > 
> > > > > > Anyways, I'm open to suggestions and can try the latter approach if
> > > > > > there are some benefits I'm not aware of or a better approach.
> > > > > 
> > > > > Coiby, there are multiple issues being discussed here.  Before deciding on an
> > > > > appropriate solution, let's frame the issues(s) properly.
> > > 
> > > Hi Mimi,
> > > 
> > > Thanks for listing and framing the issues! Sorry, it took me a while to
> > > go through your feedback as I also had a 8-day holiday.
> > > 
> > > > > 
> > > > > 1. The finit_module syscall eventually calls init_module_from_file() to read the
> > > > > module into memory and then decompress it.  The problem is that the kernel
> > > > > module signature verification occurs during the kernel_read_file(), before the
> > > > > kernel module is decompressed.  Thus, the appended kernel module signature
> > > > > cannot be verified.
> > > 
> > > Since IMA only accesses a kernel module as a fd or struct file*, even if
> > > IMA hook is triggerd after kernel module is decompressed, IMA still
> > > doesn't know the default verification has passed or can't access the
> > > decompressed kernel buffer [2] to do the verification by itself.
> > > 
> > > [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
> > > 
> > > > > 
> > > > > 2. CPIO doesn't have xattr support. There were multiple attempts at including
> > > > > xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
> > > > > security.ima were available in the initramfs, then finit_module() could verify
> > > > > them, as opposed to the appended kernel module signature.
> > > 
> > > Thanks you for pointing me to the work [1]. I'll take a more careful
> > > look at [1]. I think CPIO supporting xattr can be a long-term solution
> > > and also close a important security gap.
> > > 
> > > > > 
> > > > > 3. The issues described above are generic, not limited to Power.  When
> > > > > CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
> > > > > include an "appraise func=MODULE_CHECK".
> > > 
> > > Yes, the issue is not limited to Power. And thanks for correcting me
> > > that Power arch specific IMA policy rules don't have this problem! Sorry
> > > I misread the code.
> > > 
> > > > > 
> > > > > 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
> > > > > policy, specified on the boot command line as "ima_policy=secure_boot", always
> > > > > enforces the IMA signature stored in security.ima.
> > > > > 
> > > > > Partial solutions without kernel changes:
> > > > > - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
> > > > > - Disable kernel module compression.
> > > > > 
> > > > > Complete solution:
> > > > > - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
> > > > > - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
> > > > > enumeration is READING_MODULE and the kernel module is compressed.  The change
> > > > > might be limited to ima_post_read_file().
> > > > 
> > > > or perhaps not totally.
> > > > 
> > > > init_module_from_file() doesn't pass the flags variable to kernel_read_file().
> > > > You might want to consider defining a new kernel_read_file_id enumeration named
> > > > READING_COMPRESSED_MODULE.
> > > 
> > > Thanks for suggesting the solutions! I like the solution of CPIO
> > > supporting xattr but it seems it won't land in upstream soon. So I
> > > prefer the last approach. I've implemented one [3] by defining a new
> > > kernel_read_file_id enumeration, would you like me to post the patches
> > > to the mailing list directly?
> > > 
> > > [3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima
> > 
> > A few thoughts, before you post the patch.
> 
> Thank you for sharing your thoughts!
> 
> > 
> > 1. In the general case, the kernel module could be compressed, but without an
> > appended signature.  The new code should only defer appended signature
> > verification, if there isn't an xattr or appended signature.
> 
> I'll add these two condition checks, thanks!
> 
> > 
> > 2. Instead of defining an additional process_measurement() argument to identify
> > compressed kernel modules, to simplify the code it might be possible to define a
> > new "func" named COMPRESSED_MODULE_CHECK.
> > 
> > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> 
> I also thought about this approach. But IMA rule maps kernel module
> loading to MODULE_CHECK. If we define a new rule and ask users to use
> this new rule, ima_policy=secure_boot still won't work.

I don't have a problem with extending the "secure-boot" policy to support
uncompressed kernel modules appended signatures, based on whether
CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
MODULE_CHECK rule.

> 
> > 
> > 3.  The patch title "ima: Use default kernel module signature verification for
> > compressed ones" is a bit off.  It should be something along the lines of "ima:
> > defer compressed kernel module appended signature verification".
> 
> > 
> > 4. Simplify the patch description.
> 
> I'll rephrase the title and try simplifying it. Thanks!

Thank you.

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Fri, Oct 17, 2025 at 01:49:58PM -0400, Mimi Zohar wrote:
>On Fri, 2025-10-17 at 11:19 +0800, Coiby Xu wrote:
>> On Thu, Oct 16, 2025 at 10:31:35PM -0400, Mimi Zohar wrote:
>> > On Thu, 2025-10-16 at 11:46 +0800, Coiby Xu wrote:
>> > > On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
>> > > > On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
>> > > > > On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
>> > > > > > Currently, for any IMA policy that requires appraisal for kernel modules
>> > > > > > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
>> > > > > > booting will fail because IMA will reject a kernel module which will
>> > > > > > be decompressed in the kernel space and then have its signature
>> > > > > > verified.
>> > > > > >
>> > > > > > This happens because when in-kernel module decompression
>> > > > > > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
>> > > > > > syscall instead of init_module to load a module. And IMA mandates IMA
>> > > > > > xattr verification for finit_module unless appraise_type=imasig|modsig
>> > > > > > is specified in the rule.  However currently initramfs doesn't support
>> > > > > > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
>> > > > > > doesn't work either because IMA will treat to-be-decompressed kernel
>> > > > > > module as not having module signature as it can't decompress kernel
>> > > > > > module to check if signature exists.
>> > > > > >
>> > > > > > So fall back to default kernel module signature verification when we have
>> > > > > > no way to verify IMA xattr.
>> > > > > >
>> > > > > > Reported-by: Karel Srot <ksrot@redhat.com>
>> > > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > > > > ---
>> > > > > > Another approach will be to make IMA decompress the kernel module to
>> > > > > > check the signature. This requires refactoring kernel module code to
>> > > > > > make the in-kernel module decompressing feature modular and seemingly
>> > > > > > more efforts are needed. A second disadvantage is it feels
>> > > > > > counter-intuitive to verify the same kernel module signature twice. And
>> > > > > > we still need to make ima_policy=secure_boot allow verifying appended
>> > > > > > module signature.
>> > > > > >
>> > > > > > Anyways, I'm open to suggestions and can try the latter approach if
>> > > > > > there are some benefits I'm not aware of or a better approach.
>> > > > >
>> > > > > Coiby, there are multiple issues being discussed here.  Before deciding on an
>> > > > > appropriate solution, let's frame the issues(s) properly.
>> > >
>> > > Hi Mimi,
>> > >
>> > > Thanks for listing and framing the issues! Sorry, it took me a while to
>> > > go through your feedback as I also had a 8-day holiday.
>> > >
>> > > > >
>> > > > > 1. The finit_module syscall eventually calls init_module_from_file() to read the
>> > > > > module into memory and then decompress it.  The problem is that the kernel
>> > > > > module signature verification occurs during the kernel_read_file(), before the
>> > > > > kernel module is decompressed.  Thus, the appended kernel module signature
>> > > > > cannot be verified.
>> > >
>> > > Since IMA only accesses a kernel module as a fd or struct file*, even if
>> > > IMA hook is triggerd after kernel module is decompressed, IMA still
>> > > doesn't know the default verification has passed or can't access the
>> > > decompressed kernel buffer [2] to do the verification by itself.
>> > >
>> > > [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
>> > >
>> > > > >
>> > > > > 2. CPIO doesn't have xattr support. There were multiple attempts at including
>> > > > > xattrs in CPIO, but none were upstreamed [1].  If file signatures stored in
>> > > > > security.ima were available in the initramfs, then finit_module() could verify
>> > > > > them, as opposed to the appended kernel module signature.
>> > >
>> > > Thanks you for pointing me to the work [1]. I'll take a more careful
>> > > look at [1]. I think CPIO supporting xattr can be a long-term solution
>> > > and also close a important security gap.
>> > >
>> > > > >
>> > > > > 3. The issues described above are generic, not limited to Power.  When
>> > > > > CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
>> > > > > include an "appraise func=MODULE_CHECK".
>> > >
>> > > Yes, the issue is not limited to Power. And thanks for correcting me
>> > > that Power arch specific IMA policy rules don't have this problem! Sorry
>> > > I misread the code.
>> > >
>> > > > >
>> > > > > 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
>> > > > > policy, specified on the boot command line as "ima_policy=secure_boot", always
>> > > > > enforces the IMA signature stored in security.ima.
>> > > > >
>> > > > > Partial solutions without kernel changes:
>> > > > > - Enable CONFIG_MODULE_SIG  (Doesn't solve 4)
>> > > > > - Disable kernel module compression.
>> > > > >
>> > > > > Complete solution:
>> > > > > - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
>> > > > > - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
>> > > > > enumeration is READING_MODULE and the kernel module is compressed.  The change
>> > > > > might be limited to ima_post_read_file().
>> > > >
>> > > > or perhaps not totally.
>> > > >
>> > > > init_module_from_file() doesn't pass the flags variable to kernel_read_file().
>> > > > You might want to consider defining a new kernel_read_file_id enumeration named
>> > > > READING_COMPRESSED_MODULE.
>> > >
>> > > Thanks for suggesting the solutions! I like the solution of CPIO
>> > > supporting xattr but it seems it won't land in upstream soon. So I
>> > > prefer the last approach. I've implemented one [3] by defining a new
>> > > kernel_read_file_id enumeration, would you like me to post the patches
>> > > to the mailing list directly?
>> > >
>> > > [3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima
>> >
>> > A few thoughts, before you post the patch.
>>
>> Thank you for sharing your thoughts!
>>
>> >
>> > 1. In the general case, the kernel module could be compressed, but without an
>> > appended signature.  The new code should only defer appended signature
>> > verification, if there isn't an xattr or appended signature.
>>
>> I'll add these two condition checks, thanks!
>>
>> >
>> > 2. Instead of defining an additional process_measurement() argument to identify
>> > compressed kernel modules, to simplify the code it might be possible to define a
>> > new "func" named COMPRESSED_MODULE_CHECK.
>> >
>> > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
>>
>> I also thought about this approach. But IMA rule maps kernel module
>> loading to MODULE_CHECK. If we define a new rule and ask users to use
>> this new rule, ima_policy=secure_boot still won't work.
>
>I don't have a problem with extending the "secure-boot" policy to support
>uncompressed kernel modules appended signatures, based on whether
>CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
>MODULE_CHECK rule.

I assume once the new rule get added, we can't remove it for userspace
backward compatibility, right? And with CPIO xattr supported, it seems
there is no need to keep this rule. So if this concern is valid, do you
think we shall switch to another approach i.e. to make IMA support
verifying decompressed module and then make "secure-boot" to allow
appended module signature?

Another thought is to make CPIO support xattr. Today I realize that
ima_policy=secure_boot can also cause failure of loading kdump kernel.
So the issue this patch tries to resolves has much less impact than I
thought. Maybe we can wait until CPIO xattr support is ready? I'll help
review and test Roberto's patches if this is the best way forward.

>
>>
>> >
>> > 3.  The patch title "ima: Use default kernel module signature verification for
>> > compressed ones" is a bit off.  It should be something along the lines of "ima:
>> > defer compressed kernel module appended signature verification".
>>
>> >
>> > 4. Simplify the patch description.
>>
>> I'll rephrase the title and try simplifying it. Thanks!
>
>Thank you.
>

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > 
> > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> > > 
> > > I also thought about this approach. But IMA rule maps kernel module
> > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > this new rule, ima_policy=secure_boot still won't work.
> > 
> > I don't have a problem with extending the "secure-boot" policy to support
> > uncompressed kernel modules appended signatures, based on whether
> > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
> > MODULE_CHECK rule.
> 
> I assume once the new rule get added, we can't remove it for userspace
> backward compatibility, right? And with CPIO xattr supported, it seems
> there is no need to keep this rule. So if this concern is valid, do you
> think we shall switch to another approach i.e. to make IMA support
> verifying decompressed module and then make "secure-boot" to allow
> appended module signature?

Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
support verifying decompressed module", yes that might be a better solution,
than relying on "sig_enforce" being enabled. IMA already supports verifying the
appended signatures.  A new IMA specific or LSM hook would need to be defined
after module_decompress().

Remember based on policy, IMA supports:
1. verifying the signature stored in security.ima xattr
2. verifying the appended signature (not for compressed kernel modules)
3. verifying both the xattr and appended signatures
4. none

To prevent 3 - verifying both types of signatures, the IMA arch specific policy
rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
NOT enabled.  Calling set_module_sig_enforced() from ima_appraise_measurement()
to set sig_enforce could inadvertently result in requiring both the xattr and
the appended signature kernel module verification.  To prevent this from
happening, "sig_enforce" should not be set, only verified in
ima_appraise_measurement().

> 
> Another thought is to make CPIO support xattr. Today I realize that
> ima_policy=secure_boot can also cause failure of loading kdump kernel.
> So the issue this patch tries to resolves has much less impact than I
> thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> review and test Roberto's patches if this is the best way forward.

I'm not sure of the status of the CPIO patch set.  Roberto?

Mimi

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Roberto Sassu]

On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > > 
> > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> > > > 
> > > > I also thought about this approach. But IMA rule maps kernel module
> > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > this new rule, ima_policy=secure_boot still won't work.
> > > 
> > > I don't have a problem with extending the "secure-boot" policy to support
> > > uncompressed kernel modules appended signatures, based on whether
> > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
> > > MODULE_CHECK rule.
> > 
> > I assume once the new rule get added, we can't remove it for userspace
> > backward compatibility, right? And with CPIO xattr supported, it seems
> > there is no need to keep this rule. So if this concern is valid, do you
> > think we shall switch to another approach i.e. to make IMA support
> > verifying decompressed module and then make "secure-boot" to allow
> > appended module signature?
> 
> Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
> support verifying decompressed module", yes that might be a better solution,
> than relying on "sig_enforce" being enabled. IMA already supports verifying the
> appended signatures.  A new IMA specific or LSM hook would need to be defined
> after module_decompress().
> 
> Remember based on policy, IMA supports:
> 1. verifying the signature stored in security.ima xattr
> 2. verifying the appended signature (not for compressed kernel modules)
> 3. verifying both the xattr and appended signatures
> 4. none
> 
> To prevent 3 - verifying both types of signatures, the IMA arch specific policy
> rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
> NOT enabled.  Calling set_module_sig_enforced() from ima_appraise_measurement()
> to set sig_enforce could inadvertently result in requiring both the xattr and
> the appended signature kernel module verification.  To prevent this from
> happening, "sig_enforce" should not be set, only verified in
> ima_appraise_measurement().
> 
> > 
> > Another thought is to make CPIO support xattr. Today I realize that
> > ima_policy=secure_boot can also cause failure of loading kdump kernel.
> > So the issue this patch tries to resolves has much less impact than I
> > thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> > review and test Roberto's patches if this is the best way forward.
> 
> I'm not sure of the status of the CPIO patch set.  Roberto?

I haven't had time to look at it recently. I can take the openEuler
version, address the remaining comments and repost.

Roberto

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Mon, 2025-10-20 at 14:45 +0200, Roberto Sassu wrote:
> On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> > On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > > > 
> > > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> > > > > 
> > > > > I also thought about this approach. But IMA rule maps kernel module
> > > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > > this new rule, ima_policy=secure_boot still won't work.
> > > > 
> > > > I don't have a problem with extending the "secure-boot" policy to support
> > > > uncompressed kernel modules appended signatures, based on whether
> > > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
> > > > MODULE_CHECK rule.
> > > 
> > > I assume once the new rule get added, we can't remove it for userspace
> > > backward compatibility, right? And with CPIO xattr supported, it seems
> > > there is no need to keep this rule. So if this concern is valid, do you
> > > think we shall switch to another approach i.e. to make IMA support
> > > verifying decompressed module and then make "secure-boot" to allow
> > > appended module signature?
> > 
> > Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
> > support verifying decompressed module", yes that might be a better solution,
> > than relying on "sig_enforce" being enabled. IMA already supports verifying the
> > appended signatures.  A new IMA specific or LSM hook would need to be defined
> > after module_decompress().
> > 
> > Remember based on policy, IMA supports:
> > 1. verifying the signature stored in security.ima xattr
> > 2. verifying the appended signature (not for compressed kernel modules)
> > 3. verifying both the xattr and appended signatures
> > 4. none
> > 
> > To prevent 3 - verifying both types of signatures, the IMA arch specific policy
> > rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
> > NOT enabled.  Calling set_module_sig_enforced() from ima_appraise_measurement()
> > to set sig_enforce could inadvertently result in requiring both the xattr and
> > the appended signature kernel module verification.  To prevent this from
> > happening, "sig_enforce" should not be set, only verified in
> > ima_appraise_measurement().
> > 
> > > 
> > > Another thought is to make CPIO support xattr. Today I realize that
> > > ima_policy=secure_boot can also cause failure of loading kdump kernel.
> > > So the issue this patch tries to resolves has much less impact than I
> > > thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> > > review and test Roberto's patches if this is the best way forward.
> > 
> > I'm not sure of the status of the CPIO patch set.  Roberto?
> 
> I haven't had time to look at it recently. I can take the openEuler
> version, address the remaining comments and repost.

Thank you!

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Mon, Oct 20, 2025 at 09:57:19AM -0400, Mimi Zohar wrote:
>On Mon, 2025-10-20 at 14:45 +0200, Roberto Sassu wrote:
>> On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
[...]
>> >
>> > >
>> > > Another thought is to make CPIO support xattr. Today I realize that
>> > > ima_policy=secure_boot can also cause failure of loading kdump kernel.
>> > > So the issue this patch tries to resolves has much less impact than I
>> > > thought. Maybe we can wait until CPIO xattr support is ready? I'll help
>> > > review and test Roberto's patches if this is the best way forward.
>> >
>> > I'm not sure of the status of the CPIO patch set.  Roberto?
>>
>> I haven't had time to look at it recently. I can take the openEuler
>> version, address the remaining comments and repost.
>
>Thank you!

+1, I'm looking forward to the reposted patch set!


-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > > 
> > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> > > > 
> > > > I also thought about this approach. But IMA rule maps kernel module
> > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > this new rule, ima_policy=secure_boot still won't work.
> > > 
> > > I don't have a problem with extending the "secure-boot" policy to support
> > > uncompressed kernel modules appended signatures, based on whether
> > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
> > > MODULE_CHECK rule.
> > 
> > I assume once the new rule get added, we can't remove it for userspace
> > backward compatibility, right? And with CPIO xattr supported, it seems
> > there is no need to keep this rule. So if this concern is valid, do you
> > think we shall switch to another approach i.e. to make IMA support
> > verifying decompressed module and then make "secure-boot" to allow
> > appended module signature?
> 
> Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
> support verifying decompressed module", yes that might be a better solution,
> than relying on "sig_enforce" being enabled. IMA already supports verifying the
> appended signatures.  A new IMA specific or LSM hook would need to be defined
> after module_decompress().

Looking at the code further, decompressing the kernel module in IMA is
redundant.  Instead I think the best approach would be to:
- define DECOMPRESSED_MODULE, in addition to COMPRESSED_MODULE.

id(COMPRESSED_MODULE, compressed-kernel-module) \
id(DECOMPRESSED_MODULE, decompressed-kernel-module)    \

- instead of passing a boolean indicating whether the module is compressed, pass
the kernel_read_file_id enumeration to differentiate between the compressed and
decompressed module.

- define a new IMA hook, probably LSM hook as well, named
ima_decompressed_module().

- call the new ima_decompressed_module() from init_module_from_file()
immediately after decompressing the kernel module.  Something along the lines
of:

err = ima_decompressed_module(f, (char *)info.hdr, info.len,
                              READING_DECOMPRESSED_MODULE);

For testing purposes to see the decompressed appended signature in the
measurement list, modify the MODULE_CHECK measure rule to include "template=ima-
modsig" in ima_efi.c.

-- 
Mimi

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Fri, Oct 24, 2025 at 11:16:37AM -0400, Mimi Zohar wrote:
>On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
>> On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
>> > > > > 2. Instead of defining an additional process_measurement() argument to identify
>> > > > > compressed kernel modules, to simplify the code it might be possible to define a
>> > > > > new "func" named COMPRESSED_MODULE_CHECK.
>> > > > >
>> > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
>> > > >
>> > > > I also thought about this approach. But IMA rule maps kernel module
>> > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
>> > > > this new rule, ima_policy=secure_boot still won't work.
>> > >
>> > > I don't have a problem with extending the "secure-boot" policy to support
>> > > uncompressed kernel modules appended signatures, based on whether
>> > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
>> > > MODULE_CHECK rule.
>> >
>> > I assume once the new rule get added, we can't remove it for userspace
>> > backward compatibility, right? And with CPIO xattr supported, it seems
>> > there is no need to keep this rule. So if this concern is valid, do you
>> > think we shall switch to another approach i.e. to make IMA support
>> > verifying decompressed module and then make "secure-boot" to allow
>> > appended module signature?
>>
>> Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
>> support verifying decompressed module", yes that might be a better solution,
>> than relying on "sig_enforce" being enabled. IMA already supports verifying the
>> appended signatures.  A new IMA specific or LSM hook would need to be defined
>> after module_decompress().
>
>Looking at the code further, decompressing the kernel module in IMA is
>redundant.  Instead I think the best approach would be to:
>- define DECOMPRESSED_MODULE, in addition to COMPRESSED_MODULE.
>
>id(COMPRESSED_MODULE, compressed-kernel-module) \
>id(DECOMPRESSED_MODULE, decompressed-kernel-module)    \
>
>- instead of passing a boolean indicating whether the module is compressed, pass
>the kernel_read_file_id enumeration to differentiate between the compressed and
>decompressed module.
>
>- define a new IMA hook, probably LSM hook as well, named
>ima_decompressed_module().
>
>- call the new ima_decompressed_module() from init_module_from_file()
>immediately after decompressing the kernel module.  Something along the lines
>of:
>
>err = ima_decompressed_module(f, (char *)info.hdr, info.len,
>                              READING_DECOMPRESSED_MODULE);

Thanks for proposing a better solution! Yeah, decompressing the kernel
module in IMA is unnecessary. Do you think we can further extend your
idea to call one IMA hook only after kernel module decompression is
done? If we call two IMA hooks in finit_module, we'll need coordination
between two IMA hooks i.e. the 1st IMA hook shouldn't fail in order for
the 2nd IMA hook to be executed. The new IMA hook will always have
access to the decompressed kernel module buffer so there is no need to
differentiate between compressed and decompressed module.

Another question is whether we should allow loading a kernel module with
appended signature but misses IMA signature. Both IMA arch specific policy
and init_module syscall only require appended signature verification. On
the other hand, we only allow "appraise_type=imasig|modsig" but not
appraise_type=modsig. How about we allow loading a kernel module with
valid appended signature regardless of its IMA signature? We won't call
set_module_sig_enforced but as long as we know is_module_sig_enforced()
is true, we allow the module in IMA.


>
>For testing purposes to see the decompressed appended signature in the
>measurement list, modify the MODULE_CHECK measure rule to include "template=ima-
>modsig" in ima_efi.c.

I haven't figured out why to include "template=ima-modsig" for testing
purpose considering we can simply check if the kernel module has been
loaded successfully. It it related to the design that "The d-modsig and
modsig fields are only populated if both the measure and appraise rules
trigger"? If so, can you also help me understand there is such a design?

[1] https://ima-doc.readthedocs.io/en/latest/event-log-format.html#ima-modsig


>-- 
>Mimi
>

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Thu, 2025-10-30 at 08:31 +0800, Coiby Xu wrote:
> On Fri, Oct 24, 2025 at 11:16:37AM -0400, Mimi Zohar wrote:
> > On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> > > On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > > > > 
> > > > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
> > > > > > 
> > > > > > I also thought about this approach. But IMA rule maps kernel module
> > > > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > > > this new rule, ima_policy=secure_boot still won't work.
> > > > > 
> > > > > I don't have a problem with extending the "secure-boot" policy to support
> > > > > uncompressed kernel modules appended signatures, based on whether
> > > > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
> > > > > MODULE_CHECK rule.
> > > > 
> > > > I assume once the new rule get added, we can't remove it for userspace
> > > > backward compatibility, right? And with CPIO xattr supported, it seems
> > > > there is no need to keep this rule. So if this concern is valid, do you
> > > > think we shall switch to another approach i.e. to make IMA support
> > > > verifying decompressed module and then make "secure-boot" to allow
> > > > appended module signature?
> > > 
> > > Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
> > > support verifying decompressed module", yes that might be a better solution,
> > > than relying on "sig_enforce" being enabled. IMA already supports verifying the
> > > appended signatures.  A new IMA specific or LSM hook would need to be defined
> > > after module_decompress().
> > 
> > Looking at the code further, decompressing the kernel module in IMA is
> > redundant.  Instead I think the best approach would be to:
> > - define DECOMPRESSED_MODULE, in addition to COMPRESSED_MODULE.
> > 
> > id(COMPRESSED_MODULE, compressed-kernel-module) \
> > id(DECOMPRESSED_MODULE, decompressed-kernel-module)    \
> > 
> > - instead of passing a boolean indicating whether the module is compressed, pass
> > the kernel_read_file_id enumeration to differentiate between the compressed and
> > decompressed module.
> > 
> > - define a new IMA hook, probably LSM hook as well, named
> > ima_decompressed_module().
> > 
> > - call the new ima_decompressed_module() from init_module_from_file()
> > immediately after decompressing the kernel module.  Something along the lines
> > of:
> > 
> > err = ima_decompressed_module(f, (char *)info.hdr, info.len,
> >                              READING_DECOMPRESSED_MODULE);
> 
> Thanks for proposing a better solution! Yeah, decompressing the kernel
> module in IMA is unnecessary. Do you think we can further extend your
> idea to call one IMA hook only after kernel module decompression is
> done? If we call two IMA hooks in finit_module, we'll need coordination
> between two IMA hooks i.e. the 1st IMA hook shouldn't fail in order for
> the 2nd IMA hook to be executed. The new IMA hook will always have
> access to the decompressed kernel module buffer so there is no need to
> differentiate between compressed and decompressed module.

Agreed instead of verifying the kernel module signature on the LSM
kernel_post_read_file() hook, define and move it to a new IMA/LSM hook after it
is decompressed, would be much simpler than coordinating two LSM hooks.

> 
> Another question is whether we should allow loading a kernel module with
> appended signature but misses IMA signature. Both IMA arch specific policy
> and init_module syscall only require appended signature verification. On
> the other hand, we only allow "appraise_type=imasig|modsig" but not
> appraise_type=modsig. How about we allow loading a kernel module with
> valid appended signature regardless of its IMA signature? We won't call
> set_module_sig_enforced but as long as we know is_module_sig_enforced()
> is true, we allow the module in IMA.

Based on the policy, IMA enforces signature verification. Only if
CONFIG_MODULE_SIG is configured, the IMA arch specific policy does not define an
IMA kernel module appraise rule. However, custom policies could still require
both types of signatures, not necessarily signed by the same entity.

The option "appraise_type=imasig|modsig" allows either an IMA signature OR an
appended signature.

> 
> > 
> > For testing purposes to see the decompressed appended signature in the
> > measurement list, modify the MODULE_CHECK measure rule to include "template=ima-
> > modsig" in ima_efi.c.
> 
> I haven't figured out why to include "template=ima-modsig" for testing
> purpose considering we can simply check if the kernel module has been
> loaded successfully.

That's fine too.

> It it related to the design that "The d-modsig and
> modsig fields are only populated if both the measure and appraise rules
> trigger"? If so, can you also help me understand there is such a design?
> 
> [1] https://ima-doc.readthedocs.io/en/latest/event-log-format.html#ima-modsig

The "ima-sig" template contains the file data hash and file signature, allowing
the attestation server to verify the signature based on the file data hash
contained in the measurement list.

In addition to the file data hash and the file signature, the "ima-modsig"
template contains the file data hash without the appended signature, allowing
the attestation server to verify the appended signature against it.

Mimi

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Wed, Oct 29, 2025 at 11:01:27PM -0400, Mimi Zohar wrote:
>On Thu, 2025-10-30 at 08:31 +0800, Coiby Xu wrote:
>> On Fri, Oct 24, 2025 at 11:16:37AM -0400, Mimi Zohar wrote:
>> > On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
>> > > On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
>> > > > > > > 2. Instead of defining an additional process_measurement() argument to identify
>> > > > > > > compressed kernel modules, to simplify the code it might be possible to define a
>> > > > > > > new "func" named COMPRESSED_MODULE_CHECK.
>> > > > > > >
>> > > > > > > +       [READING_COMPRESSED_MODULE] = MODULE_CHECK,  -> COMPRESSED_MODULE_CHECK
>> > > > > >
>> > > > > > I also thought about this approach. But IMA rule maps kernel module
>> > > > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
>> > > > > > this new rule, ima_policy=secure_boot still won't work.
>> > > > >
>> > > > > I don't have a problem with extending the "secure-boot" policy to support
>> > > > > uncompressed kernel modules appended signatures, based on whether
>> > > > > CONFIG_MODULE_SIG is enabled.  The new rule would be in addition to the existing
>> > > > > MODULE_CHECK rule.
>> > > >
>> > > > I assume once the new rule get added, we can't remove it for userspace
>> > > > backward compatibility, right? And with CPIO xattr supported, it seems
>> > > > there is no need to keep this rule. So if this concern is valid, do you
>> > > > think we shall switch to another approach i.e. to make IMA support
>> > > > verifying decompressed module and then make "secure-boot" to allow
>> > > > appended module signature?
>> > >
>> > > Yes, once the rule is added, it wouldn't be removed.  As for "to make IMA
>> > > support verifying decompressed module", yes that might be a better solution,
>> > > than relying on "sig_enforce" being enabled. IMA already supports verifying the
>> > > appended signatures.  A new IMA specific or LSM hook would need to be defined
>> > > after module_decompress().
>> >
>> > Looking at the code further, decompressing the kernel module in IMA is
>> > redundant.  Instead I think the best approach would be to:
>> > - define DECOMPRESSED_MODULE, in addition to COMPRESSED_MODULE.
>> >
>> > id(COMPRESSED_MODULE, compressed-kernel-module) \
>> > id(DECOMPRESSED_MODULE, decompressed-kernel-module)    \
>> >
>> > - instead of passing a boolean indicating whether the module is compressed, pass
>> > the kernel_read_file_id enumeration to differentiate between the compressed and
>> > decompressed module.
>> >
>> > - define a new IMA hook, probably LSM hook as well, named
>> > ima_decompressed_module().
>> >
>> > - call the new ima_decompressed_module() from init_module_from_file()
>> > immediately after decompressing the kernel module.  Something along the lines
>> > of:
>> >
>> > err = ima_decompressed_module(f, (char *)info.hdr, info.len,
>> >                              READING_DECOMPRESSED_MODULE);
>>
>> Thanks for proposing a better solution! Yeah, decompressing the kernel
>> module in IMA is unnecessary. Do you think we can further extend your
>> idea to call one IMA hook only after kernel module decompression is
>> done? If we call two IMA hooks in finit_module, we'll need coordination
>> between two IMA hooks i.e. the 1st IMA hook shouldn't fail in order for
>> the 2nd IMA hook to be executed. The new IMA hook will always have
>> access to the decompressed kernel module buffer so there is no need to
>> differentiate between compressed and decompressed module.
>
>Agreed instead of verifying the kernel module signature on the LSM
>kernel_post_read_file() hook, define and move it to a new IMA/LSM hook after it
>is decompressed, would be much simpler than coordinating two LSM hooks.

Thanks for confirming it! I'll send a new version once the testing is
finished.

>
>>
>> Another question is whether we should allow loading a kernel module with
>> appended signature but misses IMA signature. Both IMA arch specific policy
>> and init_module syscall only require appended signature verification. On
>> the other hand, we only allow "appraise_type=imasig|modsig" but not
>> appraise_type=modsig. How about we allow loading a kernel module with
>> valid appended signature regardless of its IMA signature? We won't call
>> set_module_sig_enforced but as long as we know is_module_sig_enforced()
>> is true, we allow the module in IMA.
>
>Based on the policy, IMA enforces signature verification. Only if
>CONFIG_MODULE_SIG is configured, the IMA arch specific policy does not define an
>IMA kernel module appraise rule. However, custom policies could still require
>both types of signatures, not necessarily signed by the same entity.
>
>The option "appraise_type=imasig|modsig" allows either an IMA signature OR an
>appended signature.

Thanks for the clarification! If I understand you correctly, some users
may want to enforce IMA signature verification and we should provide
such flexibility. Then do you think it's a good idea to change the kernel
module rule in ima_policy=secure_boot to 
"appraise func=MODULE_CHECK appraise_type=imasig|modsig" so
ima_policy=secure_boot can also work for in-kernel decompressing
modules?

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Mimi Zohar]

On Thu, 2025-10-30 at 21:42 +0800, Coiby Xu wrote:
> > > 
> > > Another question is whether we should allow loading a kernel module with
> > > appended signature but misses IMA signature. Both IMA arch specific policy
> > > and init_module syscall only require appended signature verification. On
> > > the other hand, we only allow "appraise_type=imasig|modsig" but not
> > > appraise_type=modsig. How about we allow loading a kernel module with
> > > valid appended signature regardless of its IMA signature? We won't call
> > > set_module_sig_enforced but as long as we know is_module_sig_enforced()
> > > is true, we allow the module in IMA.
> > 
> > Based on the policy, IMA enforces signature verification. Only if
> > CONFIG_MODULE_SIG is configured, the IMA arch specific policy does not define an
> > IMA kernel module appraise rule. However, custom policies could still require
> > both types of signatures, not necessarily signed by the same entity.
> > 
> > The option "appraise_type=imasig|modsig" allows either an IMA signature OR an
> > appended signature.
> 
> Thanks for the clarification! If I understand you correctly, some users
> may want to enforce IMA signature verification and we should provide
> such flexibility. Then do you think it's a good idea to change the kernel
> module rule in ima_policy=secure_boot to 
> "appraise func=MODULE_CHECK appraise_type=imasig|modsig" so
> ima_policy=secure_boot can also work for in-kernel decompressing
> modules?

Yes, that's fine.  Unlike the arch specific policy rules and the Kconfig
appraise rules, which persist after loading a custom policy, the builtin secure
boot rules do not persist.

Mimi

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Thu, Oct 30, 2025 at 12:50:48PM -0400, Mimi Zohar wrote:
>On Thu, 2025-10-30 at 21:42 +0800, Coiby Xu wrote:
>> > >
>> > > Another question is whether we should allow loading a kernel module with
>> > > appended signature but misses IMA signature. Both IMA arch specific policy
>> > > and init_module syscall only require appended signature verification. On
>> > > the other hand, we only allow "appraise_type=imasig|modsig" but not
>> > > appraise_type=modsig. How about we allow loading a kernel module with
>> > > valid appended signature regardless of its IMA signature? We won't call
>> > > set_module_sig_enforced but as long as we know is_module_sig_enforced()
>> > > is true, we allow the module in IMA.
>> >
>> > Based on the policy, IMA enforces signature verification. Only if
>> > CONFIG_MODULE_SIG is configured, the IMA arch specific policy does not define an
>> > IMA kernel module appraise rule. However, custom policies could still require
>> > both types of signatures, not necessarily signed by the same entity.
>> >
>> > The option "appraise_type=imasig|modsig" allows either an IMA signature OR an
>> > appended signature.
>>
>> Thanks for the clarification! If I understand you correctly, some users
>> may want to enforce IMA signature verification and we should provide
>> such flexibility. Then do you think it's a good idea to change the kernel
>> module rule in ima_policy=secure_boot to
>> "appraise func=MODULE_CHECK appraise_type=imasig|modsig" so
>> ima_policy=secure_boot can also work for in-kernel decompressing
>> modules?
>
>Yes, that's fine.  Unlike the arch specific policy rules and the Kconfig
>appraise rules, which persist after loading a custom policy, the builtin secure
>boot rules do not persist.

Thanks for the confirmation! v2 has been posted.

>
>Mimi
>
>

-- 
Best regards,
Coiby

Re: [PATCH] ima: Fall back to default kernel module signature verification

[kernel test robot]

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on cec1e6e5d1ab33403b809f79cd20d6aff124ccfe]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/ima-Fall-back-to-default-kernel-module-signature-verification/20250928-110501
base:   cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
patch link:    https://lore.kernel.org/r/20250928030358.3873311-1-coxu%40redhat.com
patch subject: [PATCH] ima: Fall back to default kernel module signature verification
config: i386-randconfig-012-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'


vim +587 security/integrity/ima/ima_appraise.c

   483	
   484	/*
   485	 * ima_appraise_measurement - appraise file measurement
   486	 *
   487	 * Call evm_verifyxattr() to verify the integrity of 'security.ima'.
   488	 * Assuming success, compare the xattr hash with the collected measurement.
   489	 *
   490	 * Return 0 on success, error code otherwise
   491	 */
   492	int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
   493				     struct file *file, const unsigned char *filename,
   494				     struct evm_ima_xattr_data *xattr_value,
   495				     int xattr_len, const struct modsig *modsig)
   496	{
   497		static const char op[] = "appraise_data";
   498		int audit_msgno = AUDIT_INTEGRITY_DATA;
   499		const char *cause = "unknown";
   500		struct dentry *dentry = file_dentry(file);
   501		struct inode *inode = d_backing_inode(dentry);
   502		enum integrity_status status = INTEGRITY_UNKNOWN;
   503		int rc = xattr_len;
   504		bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
   505		bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
   506	
   507		/* If not appraising a modsig or using default module verification, we need an xattr. */
   508		if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
   509			return INTEGRITY_UNKNOWN;
   510	
   511		/*
   512		 * Unlike any of the other LSM hooks where the kernel enforces file
   513		 * integrity, enforcing file integrity for the bprm_creds_for_exec()
   514		 * LSM hook with the AT_EXECVE_CHECK flag is left up to the discretion
   515		 * of the script interpreter(userspace). Differentiate kernel and
   516		 * userspace enforced integrity audit messages.
   517		 */
   518		if (is_bprm_creds_for_exec(func, file))
   519			audit_msgno = AUDIT_INTEGRITY_USERSPACE;
   520	
   521		/* If reading the xattr failed and there's no modsig or module verification, error out. */
   522		if (rc <= 0 && !try_modsig && !enforce_module_sig) {
   523			if (rc && rc != -ENODATA)
   524				goto out;
   525	
   526			if (iint->flags & IMA_DIGSIG_REQUIRED) {
   527				if (iint->flags & IMA_VERITY_REQUIRED)
   528					cause = "verity-signature-required";
   529				else
   530					cause = "IMA-signature-required";
   531			} else {
   532				cause = "missing-hash";
   533			}
   534	
   535			status = INTEGRITY_NOLABEL;
   536			if (file->f_mode & FMODE_CREATED)
   537				iint->flags |= IMA_NEW_FILE;
   538			if ((iint->flags & IMA_NEW_FILE) &&
   539			    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
   540			     (inode->i_size == 0)))
   541				status = INTEGRITY_PASS;
   542			goto out;
   543		}
   544	
   545		status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value,
   546					 rc < 0 ? 0 : rc);
   547		switch (status) {
   548		case INTEGRITY_PASS:
   549		case INTEGRITY_PASS_IMMUTABLE:
   550		case INTEGRITY_UNKNOWN:
   551			break;
   552		case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
   553			/* Fine to not have xattrs when using a modsig or default module verification. */
   554			if (try_modsig || enforce_module_sig)
   555				break;
   556			fallthrough;
   557		case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
   558			cause = "missing-HMAC";
   559			goto out;
   560		case INTEGRITY_FAIL_IMMUTABLE:
   561			set_bit(IMA_DIGSIG, &iint->atomic_flags);
   562			cause = "invalid-fail-immutable";
   563			goto out;
   564		case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
   565			cause = "invalid-HMAC";
   566			goto out;
   567		default:
   568			WARN_ONCE(true, "Unexpected integrity status %d\n", status);
   569		}
   570	
   571		if (xattr_value)
   572			rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
   573					  &cause);
   574	
   575		/*
   576		 * If we have a modsig and either no imasig or the imasig's key isn't
   577		 * known, then try verifying the modsig.
   578		 */
   579		if (try_modsig &&
   580		    (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
   581		     rc == -ENOKEY))
   582			rc = modsig_verify(func, modsig, &status, &cause);
   583	
   584		/* Fall back to default kernel module signature verification */
   585		if (rc && enforce_module_sig) {
   586			rc = 0;
 > 587			set_module_sig_enforced();
   588			/* CONFIG_MODULE_SIG may be disabled */
   589			if (is_module_sig_enforced()) {
   590				rc = 0;
   591				status = INTEGRITY_PASS;
   592				pr_debug("Fall back to default kernel module verification for %s\n", filename);
   593			}
   594		}
   595	
   596	out:
   597		/*
   598		 * File signatures on some filesystems can not be properly verified.
   599		 * When such filesystems are mounted by an untrusted mounter or on a
   600		 * system not willing to accept such a risk, fail the file signature
   601		 * verification.
   602		 */
   603		if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
   604		    ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) ||
   605		     (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
   606			status = INTEGRITY_FAIL;
   607			cause = "unverifiable-signature";
   608			integrity_audit_msg(audit_msgno, inode, filename,
   609					    op, cause, rc, 0);
   610		} else if (status != INTEGRITY_PASS) {
   611			/* Fix mode, but don't replace file signatures. */
   612			if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
   613			    (!xattr_value ||
   614			     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
   615				if (!ima_fix_xattr(dentry, iint))
   616					status = INTEGRITY_PASS;
   617			}
   618	
   619			/*
   620			 * Permit new files with file/EVM portable signatures, but
   621			 * without data.
   622			 */
   623			if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
   624			    test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
   625				status = INTEGRITY_PASS;
   626			}
   627	
   628			integrity_audit_msg(audit_msgno, inode, filename,
   629					    op, cause, rc, 0);
   630		} else {
   631			ima_cache_flags(iint, func);
   632		}
   633	
   634		ima_set_cache_status(iint, func, status);
   635		return status;
   636	}
   637	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] ima: Fall back to default kernel module signature verification

[Coiby Xu]

On Fri, Oct 03, 2025 at 01:17:30AM +0800, kernel test robot wrote:
>Hi Coiby,

Hi,

>
>kernel test robot noticed the following build errors:
>
>[auto build test ERROR on cec1e6e5d1ab33403b809f79cd20d6aff124ccfe]
>
>url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/ima-Fall-back-to-default-kernel-module-signature-verification/20250928-110501
>base:   cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
>patch link:    https://lore.kernel.org/r/20250928030358.3873311-1-coxu%40redhat.com
>patch subject: [PATCH] ima: Fall back to default kernel module signature verification
>config: i386-randconfig-012-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/config)
>compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
>reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/reproduce)
>
>If you fix the issue in a separate patch/commit (i.e. not just a new version of
>the same patch/commit), kindly add following tags
>| Reported-by: kernel test robot <lkp@intel.com>
>| Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>
>All errors (new ones prefixed by >>):
>
>   ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'

Thanks for reporting the error! This happens when
set_module_sig_enforced is called without CONFIG_MODULE_SIG not enabled.
I'll address this issue by declaring set_module_sig_enforced only when
CONFIG_MODULE_SIG is enabled.

>
>
>vim +587 security/integrity/ima/ima_appraise.c
>
>   483	
>   484	/*
>   485	 * ima_appraise_measurement - appraise file measurement
>   486	 *
>   487	 * Call evm_verifyxattr() to verify the integrity of 'security.ima'.
>   488	 * Assuming success, compare the xattr hash with the collected measurement.
>   489	 *
>   490	 * Return 0 on success, error code otherwise
>   491	 */
>   492	int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
>   493				     struct file *file, const unsigned char *filename,
>   494				     struct evm_ima_xattr_data *xattr_value,
>   495				     int xattr_len, const struct modsig *modsig)
>   496	{
>   497		static const char op[] = "appraise_data";
>   498		int audit_msgno = AUDIT_INTEGRITY_DATA;
>   499		const char *cause = "unknown";
>   500		struct dentry *dentry = file_dentry(file);
>   501		struct inode *inode = d_backing_inode(dentry);
>   502		enum integrity_status status = INTEGRITY_UNKNOWN;
>   503		int rc = xattr_len;
>   504		bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
>   505		bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
>   506	
>   507		/* If not appraising a modsig or using default module verification, we need an xattr. */
>   508		if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
>   509			return INTEGRITY_UNKNOWN;
>   510	
>   511		/*
>   512		 * Unlike any of the other LSM hooks where the kernel enforces file
>   513		 * integrity, enforcing file integrity for the bprm_creds_for_exec()
>   514		 * LSM hook with the AT_EXECVE_CHECK flag is left up to the discretion
>   515		 * of the script interpreter(userspace). Differentiate kernel and
>   516		 * userspace enforced integrity audit messages.
>   517		 */
>   518		if (is_bprm_creds_for_exec(func, file))
>   519			audit_msgno = AUDIT_INTEGRITY_USERSPACE;
>   520	
>   521		/* If reading the xattr failed and there's no modsig or module verification, error out. */
>   522		if (rc <= 0 && !try_modsig && !enforce_module_sig) {
>   523			if (rc && rc != -ENODATA)
>   524				goto out;
>   525	
>   526			if (iint->flags & IMA_DIGSIG_REQUIRED) {
>   527				if (iint->flags & IMA_VERITY_REQUIRED)
>   528					cause = "verity-signature-required";
>   529				else
>   530					cause = "IMA-signature-required";
>   531			} else {
>   532				cause = "missing-hash";
>   533			}
>   534	
>   535			status = INTEGRITY_NOLABEL;
>   536			if (file->f_mode & FMODE_CREATED)
>   537				iint->flags |= IMA_NEW_FILE;
>   538			if ((iint->flags & IMA_NEW_FILE) &&
>   539			    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
>   540			     (inode->i_size == 0)))
>   541				status = INTEGRITY_PASS;
>   542			goto out;
>   543		}
>   544	
>   545		status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value,
>   546					 rc < 0 ? 0 : rc);
>   547		switch (status) {
>   548		case INTEGRITY_PASS:
>   549		case INTEGRITY_PASS_IMMUTABLE:
>   550		case INTEGRITY_UNKNOWN:
>   551			break;
>   552		case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
>   553			/* Fine to not have xattrs when using a modsig or default module verification. */
>   554			if (try_modsig || enforce_module_sig)
>   555				break;
>   556			fallthrough;
>   557		case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
>   558			cause = "missing-HMAC";
>   559			goto out;
>   560		case INTEGRITY_FAIL_IMMUTABLE:
>   561			set_bit(IMA_DIGSIG, &iint->atomic_flags);
>   562			cause = "invalid-fail-immutable";
>   563			goto out;
>   564		case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
>   565			cause = "invalid-HMAC";
>   566			goto out;
>   567		default:
>   568			WARN_ONCE(true, "Unexpected integrity status %d\n", status);
>   569		}
>   570	
>   571		if (xattr_value)
>   572			rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
>   573					  &cause);
>   574	
>   575		/*
>   576		 * If we have a modsig and either no imasig or the imasig's key isn't
>   577		 * known, then try verifying the modsig.
>   578		 */
>   579		if (try_modsig &&
>   580		    (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
>   581		     rc == -ENOKEY))
>   582			rc = modsig_verify(func, modsig, &status, &cause);
>   583	
>   584		/* Fall back to default kernel module signature verification */
>   585		if (rc && enforce_module_sig) {
>   586			rc = 0;
> > 587			set_module_sig_enforced();
>   588			/* CONFIG_MODULE_SIG may be disabled */
>   589			if (is_module_sig_enforced()) {
>   590				rc = 0;
>   591				status = INTEGRITY_PASS;
>   592				pr_debug("Fall back to default kernel module verification for %s\n", filename);
>   593			}
>   594		}
>   595	
>   596	out:
>   597		/*
>   598		 * File signatures on some filesystems can not be properly verified.
>   599		 * When such filesystems are mounted by an untrusted mounter or on a
>   600		 * system not willing to accept such a risk, fail the file signature
>   601		 * verification.
>   602		 */
>   603		if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
>   604		    ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) ||
>   605		     (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
>   606			status = INTEGRITY_FAIL;
>   607			cause = "unverifiable-signature";
>   608			integrity_audit_msg(audit_msgno, inode, filename,
>   609					    op, cause, rc, 0);
>   610		} else if (status != INTEGRITY_PASS) {
>   611			/* Fix mode, but don't replace file signatures. */
>   612			if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
>   613			    (!xattr_value ||
>   614			     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
>   615				if (!ima_fix_xattr(dentry, iint))
>   616					status = INTEGRITY_PASS;
>   617			}
>   618	
>   619			/*
>   620			 * Permit new files with file/EVM portable signatures, but
>   621			 * without data.
>   622			 */
>   623			if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
>   624			    test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
>   625				status = INTEGRITY_PASS;
>   626			}
>   627	
>   628			integrity_audit_msg(audit_msgno, inode, filename,
>   629					    op, cause, rc, 0);
>   630		} else {
>   631			ima_cache_flags(iint, func);
>   632		}
>   633	
>   634		ima_set_cache_status(iint, func, status);
>   635		return status;
>   636	}
>   637	
>
>-- 
>0-DAY CI Kernel Test Service
>https://github.com/intel/lkp-tests/wiki
>

-- 
Best regards,
Coiby

[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

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

[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