Message-ID: <7unk23jlwh3gy477ose6r5mvov5apxoflxmzhhpeanvrgg4ejb@h74hvym5r67s>
Date: Wed, 19 Nov 2025 11:52:15 +0800
From: Coiby Xu <coxu@...hat.com>
To: Mimi Zohar <zohar@...ux.ibm.com>
Cc: Paul Moore <paul@...l-moore.com>, linux-integrity@...r.kernel.org, 
	linux-security-module@...r.kernel.org, Karel Srot <ksrot@...hat.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, Luis Chamberlain <mcgrof@...nel.org>, 
	Petr Pavlu <petr.pavlu@...e.com>, Daniel Gomez <da.gomez@...nel.org>, 
	Sami Tolvanen <samitolvanen@...gle.com>, Roberto Sassu <roberto.sassu@...wei.com>, 
	Dmitry Kasatkin <dmitry.kasatkin@...il.com>, Eric Snowberg <eric.snowberg@...cle.com>, 
	open list <linux-kernel@...r.kernel.org>, "open list:MODULE SUPPORT" <linux-modules@...r.kernel.org>
Subject: Re: [PATCH v2] lsm,ima: new LSM hook
 security_kernel_module_read_file to access decompressed kernel module

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

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

-- 
Best regards,
Coiby

Powered by blists - more mailing lists