Re: double free in alternatives/retpoline

[Mike Rapoport <rppt@kernel.org>]

On Thu, Jun 19, 2025 at 01:31:19PM +1000, Dave Airlie wrote:
> On Thu, 19 Jun 2025 at 12:33, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > [ Adding Mike Rapoport ]
> >
> > On Wed, 18 Jun 2025 at 19:08, Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > I've just tried to boot Linux master with KASAN enabled on a laptop here, and it showing a slab UAF for apply_retpolines.
> > >
> > > I haven't had a chance to bisect yet, and unfortunately I only have a photo of the oops.
> >
> > Hmm.
> >
> > I think it's due to commit a82b26451de1 ("x86/its: explicitly manage
> > permissions for ITS pages").
> >
> > Maybe I'm mis-reading it entirely, but I think that "its_fini_core()"
> > thing is entirely bogus. It does that
> >
> >         kfree(its_pages.pages);
> >
> > but as far as I can tell, that thing is happily used later by module
> > initialization.
> >
> > Freeing the pages that have been used and marked ROX sounds like it
> > should be fine, but I think it should also do
> >
> >         its_pages.pages = NULL;
> >         its_pages->num = 0;
> >
> > so that any subsequent user that comes along due to modules or
> > whatever and does __its_alloc() will DTRT wrt the realloc().
> >
> > But I might be completely barking up the wrong tree and mis-reading
> > things entirely. PeterZ? Mike?
> 
> I wonder if the module code also needs the same treatment,

Looking more closely, there is a typo in its_alloc(), it uses CONFIG_MODULE
to choose an its_array to reallocate while it should have been using
CONFIG_MODULES.

Lukas Bulwahn sent a fix for that:

https://lore.kernel.org/all/20250616100432.22941-1-lukas.bulwahn@redhat.com
 
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 6455f7f751b3..4653881a4ab3 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -182,6 +182,7 @@ static void its_fini_core(void)
>      if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
>          its_pages_protect(&its_pages);
>      kfree(its_pages.pages);
> +    its_pages.pages = NULL;
>  }
> 
>  #ifdef CONFIG_MODULES
> (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? y
> @@ -220,6 +221,8 @@ void its_free_mod(struct module *mod)
>          execmem_free(page);
>      }
>      kfree(mod->arch.its_pages.pages);
> +    mod->arch.its_pages.pages = NULL;
> +    mod->arch.its_pages.num = 0;
>  }
>  #endif /* CONFIG_MODULES */
> 
> boots for me, but I've no idea what is required or sufficient.
> 
> Dave.
> >
> >              Linus

-- 
Sincerely yours,
Mike.