Re: [PATCH v2] module: decompress: check return value of module_extend_max_pages()

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 18/05/2026 à 16:32, Andrii Kuchmenko a écrit :
> [Vous ne recevez pas souvent de courriers de capyenglishlite@gmail.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> module_extend_max_pages() calls kvrealloc() internally and returns
> -ENOMEM on allocation failure. The return value is never checked.
> The decompression loop then continues calling module_get_next_page(),
> which writes struct page pointers into info->pages[]. When used_pages
> reaches the stale max_pages value (not updated due to the failed
> extend), a subsequent write to info->pages[used_pages++] goes out of
> bounds into adjacent heap memory.
> 
> Adjacent slab objects in the same kmalloc cache (pipe_buffer,
> seq_operations, cred) can be corrupted, potentially leading to local
> privilege escalation on kernels without SLAB_VIRTUAL mitigation.
> 
> The call order in finit_module() is:
> 
>    module_decompress()    <- vulnerable, runs FIRST
>    load_module()
>      module_sig_check()   <- signature check, runs SECOND
> 
> Decompression happens before signature verification. A crafted
> compressed module submitted via finit_module(MODULE_INIT_COMPRESSED_FILE)
> reaches this code path before any signature gate is applied. On kernels
> with module.sig_enforce=0 (default without SecureBoot) or with
> unprivileged user namespaces (Ubuntu, Debian default), this is
> reachable without CAP_SYS_MODULE.
> 
> Confirmed present in mainline (tested on v6.14-rc3).
> 
> Fix: add the missing error check after module_extend_max_pages() and
> return immediately on failure. This matches the pattern used by every
> other kvrealloc() caller in the module loading path.
> 
> Fixes: 169a58ad824d ("module: add in-kernel support for decompressing")
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrii Kuchmenko <capyenglishlite@gmail.com>
> ---
> Changes in v2:
>   - Remove unnecessary initialization of 'error' to 0 (Christophe Leroy)
>   - Remove unrelated blank line after if (error) return error (Christophe Leroy)
> 
>   kernel/module/decompress.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
> --- a/kernel/module/decompress.c
> +++ b/kernel/module/decompress.c
> @@ -XXX,9 +XXX,12 @@ int module_decompress(struct load_info *info,
>                                  const void *buf, size_t size)
>   {
>          unsigned int n_pages;
>          int error;
>          ssize_t data_size;
> 
>          n_pages = DIV_ROUND_UP(size, PAGE_SIZE) * 2;
>          error = module_extend_max_pages(info, n_pages);
> +       if (error)
> +               return error;
>          data_size = MODULE_DECOMPRESS_FN(info, buf, size);
>          if (data_size < 0) {
>                  error = data_size;
> --
> 2.39.0