Re: [PATCH] module/decompress: Adjust module_extend_max_pages() allocation type

[Kees Cook <kees@kernel.org>]

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@kernel.org>
> > ---
> > Cc: Luis Chamberlain <mcgrof@kernel.org>
> > Cc: Petr Pavlu <petr.pavlu@suse.com>
> > Cc: Sami Tolvanen <samitolvanen@google.com>
> > Cc: Daniel Gomez <da.gomez@samsung.com>
> > Cc: <linux-modules@vger.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