[<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
Powered by blists - more mailing lists