[PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
Coiby Xu
coxu at redhat.com
Wed Nov 5 00:18:25 UTC 2025
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 at 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 at 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 at redhat.com>
>> > > Suggested-by: Mimi Zohar <zohar at linux.ibm.com>
>> > > Signed-off-by: Coiby Xu <coxu at 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
More information about the Linux-security-module-archive
mailing list