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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c7e94a436a3742003e5e1155a48480d8307a9c7.camel@linux.ibm.com>
Date: Wed, 05 Nov 2025 09:07:12 -0500
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Paul Moore <paul@...l-moore.com>, Coiby Xu <coxu@...hat.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 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