linux-hardening - Re: [PATCH] module/decompress: Adjust module_extend_max

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: <202504301305.2A0CD4E8@keescook>
Date: Wed, 30 Apr 2025 13:06:24 -0700
From: Kees Cook <kees@...nel.org>
To: Petr Pavlu <petr.pavlu@...e.com>
Cc: Luis Chamberlain <mcgrof@...nel.org>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	Daniel Gomez <da.gomez@...sung.com>, linux-modules@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] module/decompress: Adjust module_extend_max_pages()
 allocation type

On Mon, Apr 28, 2025 at 12:30:03PM +0200, Petr Pavlu wrote:
> On 4/26/25 08:24, Kees Cook wrote:
> > In preparation for making the kmalloc family of allocators type aware,
> > we need to make sure that the returned type from the allocation matches
> > the type of the variable being assigned. (Before, the allocator would
> > always return "void *", which can be implicitly cast to any pointer type.)
> > 
> > The assigned type is "struct page **" but the returned type will be
> > "struct page ***". These have the same allocation size (pointer size), but
> > the types don't match. Adjust the allocation type to match the assignment.
> > 
> > Signed-off-by: Kees Cook <kees@...nel.org>
> > ---
> > Cc: Luis Chamberlain <mcgrof@...nel.org>
> > Cc: Petr Pavlu <petr.pavlu@...e.com>
> > Cc: Sami Tolvanen <samitolvanen@...gle.com>
> > Cc: Daniel Gomez <da.gomez@...sung.com>
> > Cc: <linux-modules@...r.kernel.org>
> > ---
> >  kernel/module/decompress.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
> > index 474e68f0f063..bbb2a55568cd 100644
> > --- a/kernel/module/decompress.c
> > +++ b/kernel/module/decompress.c
> > @@ -19,7 +19,7 @@ static int module_extend_max_pages(struct load_info *info, unsigned int extent)
> >  	struct page **new_pages;
> >  
> >  	new_pages = kvmalloc_array(info->max_pages + extent,
> > -				   sizeof(info->pages), GFP_KERNEL);
> > +				   sizeof(*new_pages), GFP_KERNEL);
> >  	if (!new_pages)
> >  		return -ENOMEM;
> >  
> 
> The function has a similar type mismatch a few lines below:
> 
> memcpy(new_pages, info->pages, info->max_pages * sizeof(info->pages));
> 
> The sizeof operator is used on 'struct page **', but it should be really
> on 'struct page *'.
> 
> Could you please fix this as well? For consistency with your patch,
> I suggest changing it to 'sizeof(*new_pages)'.

Actually, this whole function is basically just open-coded kvrealloc...
I will send a v2.

-- 
Kees Cook