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

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

Looks like something went wrong with that reponse, so let's try again:


Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>



Le 18/05/2026 à 18:21, Christophe Leroy (CS GROUP) a écrit :
> 
> 
> 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
> 
>