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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <h4eloauedfln56moljqb724wsiqyhk4gsggexx3tsn4o4utjeu@ieczfooqcqsf>
Date: Thu, 6 Nov 2025 21:35:08 +0800
From: Coiby Xu <coxu@...hat.com>
To: Paul Moore <paul@...l-moore.com>, Mimi Zohar <zohar@...ux.ibm.com>
Cc: 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 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@...ux.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