Re: double free in alternatives/retpoline

[Mike Rapoport <rppt@kernel.org>]

Hi Dave,

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:
> >
> > 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.

Does oops happen in core code or when a module is loaded?
Can you share the kernel config?

> > > I haven't had a chance to bisect yet, and unfortunately I only have a photo of the oops.

Could still be useful :)

> > 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().

its_fini_core() is called after all pages for the core kernel are
allocated, so there should be no reallocs that use its_pages.pages after
that. 

Modules use a different array stored in the module structure itself and
that array is freed together with the reset of the module when the module
is removed.

Obviously I overlooked something because there's that UAF splat, but it's
not that module initialization reuses the same its_pages.

> > 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,
> 
> 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.

This surely won't hurt, but I'd like to understand first why there was a
UAF spat at the first place.
 
> Dave.
> >
> >              Linus

-- 
Sincerely yours,
Mike.