Re: BUG: KASAN: vmalloc-out-of-bounds in copy_to_kernel_nofault+0xd8/0x1c8 (v6.13-rc6, PowerMac G4)

[Erhard Furtner <erhard_f@mailbox.org>]

On Wed, 22 Jan 2025 19:23:00 +0100
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:

> Le 22/01/2025 à 16:32, Christophe Leroy a écrit :
> > 
> > 
> > Le 22/01/2025 à 00:21, Erhard Furtner a écrit :  
> >> On Tue, 21 Jan 2025 23:07:25 +0100
> >> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> >>  
> >>>> Meanwhile I bisected the bug. Offending commit is:
> >>>>
> >>>>    # git bisect good
> >>>> 32913f348229c9f72dda45fc2c08c6d9dfcd3d6d is the first bad commit
> >>>> commit 32913f348229c9f72dda45fc2c08c6d9dfcd3d6d
> >>>> Author: Linus Torvalds <torvalds@linux-foundation.org>
> >>>> Date:   Mon Dec 9 10:00:25 2024 -0800
> >>>>
> >>>>       futex: fix user access on powerpc
> >>>>       The powerpc user access code is special, and unlike other 
> >>>> architectures
> >>>>       distinguishes between user access for reading and writing.
> >>>>       And commit 43a43faf5376 ("futex: improve user space accesses") 
> >>>> messed
> >>>>       that up.  It went undetected elsewhere, but caused ppc32 to 
> >>>> fail early
> >>>>       during boot, because the user access had been started with
> >>>>       user_read_access_begin(), but then finished off with just a plain
> >>>>       "user_access_end()".
> >>>>       Note that the address-masking user access helpers don't even 
> >>>> have that
> >>>>       read-vs-write distinction, so if powerpc ever wants to do address
> >>>>       masking tricks, we'll have to do some extra work for it.
> >>>>       [ Make sure to also do it for the EFAULT case, as pointed out by
> >>>>         Christophe Leroy ]
> >>>>       Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> >>>>       Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> >>>>       Link: https://eur01.safelinks.protection.outlook.com/? 
> >>>> url=https%3A%2F%2Flore.kernel.org%2Fall%2F87bjxl6b0i.fsf%40igel.home%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cb4c1dc7184f54a410a0e08dd3a7270b6%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638730985407902881%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=E5Yp9jopCPE1NFuBM8rs%2B1jXZ%2FXAaKvBGpcEP%2BaMyz0%3D&reserved=0
> >>>>       Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> >>>>
> >>>>    kernel/futex/futex.h | 4 ++--
> >>>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>>
> >>>> Indeed, reverting 32913f348229c9f72dda45fc2c08c6d9dfcd3d6d on top of 
> >>>> v6.13 makes the KASAN hit disappear.  
> >>>
> >>> That looks terribly odd.
> >>>
> >>> On G4, user_read_access_begin() and user_read_access_end() are no-op
> >>> because book3s/32 can only protect user access by kernel against write.
> >>> Read is always granted.
> >>>
> >>> So the bug must be an indirect side effect of what user_access_end()
> >>> does. user_access_end() does a sync. Would the lack of sync (once
> >>> replaced user_access_end() by user_read_access_end() ) lead to some odd
> >>> re-ordering ? Or another possibility is that user_access_end() is called
> >>> on some kernel address (I see in the description of commit 43a43faf5376
> >>> ("futex: improve user space accesses") that the replaced __get_user()
> >>> was expected to work on kernel adresses) ? Calling user_access_begin()
> >>> and user_access_end() is unexpected and there is no guard so it could
> >>> lead to strange segment settings which hides a KASAN hit. But once the
> >>> fix the issue the KASAN resurfaces ? Could this be the problem ?
> >>>
> >>> Do you have a way to reproduce the bug on QEMU ? It would enable me to
> >>> investigate it further.  
> >>
> >> Attached v6.13 .config plays nicely with qemu ttyS0 (forgot to disable 
> >> SERIAL_8250 and set SERIAL_PMACZILOG + SERIAL_PMACZILOG_CONSOLE 
> >> instead as I prefer the PCI Serial card in my G4).
> >>
> >> The KASAN hit also shows up on qemu 8.2.7 via via:
> >> qemu-system-ppc -machine mac99,via=pmu -cpu 7450 -m 2G -nographic - 
> >> append console=ttyS0 -kernel vmlinux-6.13.0-PMacG4 -hda Debian-VM_g4.img
> >>  
> > 
> > I was able to reproduce it with v6.13 with QEMU when loading test_bpf 
> > module.
> > 
> > On my side, the problem doesn't disappear when reverting of commit 
> > 32913f348229 ("futex: fix user access on powerpc")
> > 
> > I bisected it to commit e4137f08816b ("mm, kasan, kmsan: instrument 
> > copy_from/to_kernel_nofault"), which makes a lot more sense to me.
> > 
> > It might be a problem in the way patch_instruction() is implemented on 
> > powerpc, to be investigated.  
> 
> I think the problem is commit 37bc3e5fd764 ("powerpc/lib/code-patching: 
> Use alternate map for patch_instruction()")
> 
> Can you try the change below:
> 
> diff --git a/arch/powerpc/lib/code-patching.c 
> b/arch/powerpc/lib/code-patching.c
> index af97fbb3c257..8a378fc19074 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -108,7 +108,7 @@ static int text_area_cpu_up(unsigned int cpu)
>   	unsigned long addr;
>   	int err;
> 
> -	area = get_vm_area(PAGE_SIZE, VM_ALLOC);
> +	area = get_vm_area(PAGE_SIZE, 0);
>   	if (!area) {
>   		WARN_ONCE(1, "Failed to create text area for cpu %d\n",
>   			cpu);

Patch applies on v6.13 and fixes the KASAN hit on QEMU and on my PowerMac G4 DP. Thanks Christophe!

Regards,
Erhard