| Message ID | 20250414-kunit-mips-v2-1-4cf01e1a29e6@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kunit: qemu_configs: Add MIPS configurations | expand |
Hi, Thomas, On Mon, Apr 14, 2025 at 4:29 PM Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote: > > Not all tasks have an ABI associated or vDSO mapped, > for example kthreads never do. > If such a task ever ends up calling stack_top(), it will derefence the > NULL vdso pointer and crash. > > This can for example happen when using kunit: > > mips_stack_top+0x28/0xc0 > arch_pick_mmap_layout+0x190/0x220 > kunit_vm_mmap_init+0xf8/0x138 > __kunit_add_resource+0x40/0xa8 > kunit_vm_mmap+0x88/0xd8 > usercopy_test_init+0xb8/0x240 > kunit_try_run_case+0x5c/0x1a8 > kunit_generic_run_threadfn_adapter+0x28/0x50 > kthread+0x118/0x240 > ret_from_kernel_thread+0x14/0x1c > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > --- > arch/mips/kernel/process.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > index b630604c577f9ff3f2493b0f254363e499c8318c..66343cb6c1737c4217ddd8a2c3ca244fac0ef8a5 100644 > --- a/arch/mips/kernel/process.c > +++ b/arch/mips/kernel/process.c > @@ -690,9 +690,11 @@ unsigned long mips_stack_top(void) > } > > /* Space for the VDSO, data page & GIC user page */ > - top -= PAGE_ALIGN(current->thread.abi->vdso->size); > - top -= PAGE_SIZE; > - top -= mips_gic_present() ? PAGE_SIZE : 0; > + if (current->thread.abi) { > + top -= PAGE_ALIGN(current->thread.abi->vdso->size); > + top -= PAGE_SIZE; > + top -= mips_gic_present() ? PAGE_SIZE : 0; > + } I think the below code should also exist only when VDSO exists. if (current->flags & PF_RANDOMIZE) top -= VDSO_RANDOMIZE_SIZE; Huacai > > /* Space for cache colour alignment */ > if (cpu_has_dc_aliases) > > -- > 2.49.0 > >
On Mon, Apr 14, 2025 at 05:32:47PM +0800, Huacai Chen wrote: > Hi, Thomas, > > On Mon, Apr 14, 2025 at 4:29 PM Thomas Weißschuh > <thomas.weissschuh@linutronix.de> wrote: > > > > Not all tasks have an ABI associated or vDSO mapped, > > for example kthreads never do. > > If such a task ever ends up calling stack_top(), it will derefence the > > NULL vdso pointer and crash. > > > > This can for example happen when using kunit: > > > > mips_stack_top+0x28/0xc0 > > arch_pick_mmap_layout+0x190/0x220 > > kunit_vm_mmap_init+0xf8/0x138 > > __kunit_add_resource+0x40/0xa8 > > kunit_vm_mmap+0x88/0xd8 > > usercopy_test_init+0xb8/0x240 > > kunit_try_run_case+0x5c/0x1a8 > > kunit_generic_run_threadfn_adapter+0x28/0x50 > > kthread+0x118/0x240 > > ret_from_kernel_thread+0x14/0x1c > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > --- > > arch/mips/kernel/process.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c > > index b630604c577f9ff3f2493b0f254363e499c8318c..66343cb6c1737c4217ddd8a2c3ca244fac0ef8a5 100644 > > --- a/arch/mips/kernel/process.c > > +++ b/arch/mips/kernel/process.c > > @@ -690,9 +690,11 @@ unsigned long mips_stack_top(void) > > } > > > > /* Space for the VDSO, data page & GIC user page */ > > - top -= PAGE_ALIGN(current->thread.abi->vdso->size); > > - top -= PAGE_SIZE; > > - top -= mips_gic_present() ? PAGE_SIZE : 0; > > + if (current->thread.abi) { > > + top -= PAGE_ALIGN(current->thread.abi->vdso->size); > > + top -= PAGE_SIZE; > > + top -= mips_gic_present() ? PAGE_SIZE : 0; > > + } > I think the below code should also exist only when VDSO exists. > > if (current->flags & PF_RANDOMIZE) > top -= VDSO_RANDOMIZE_SIZE; Good point, thanks. I'll move that up into the same new conditional block. > Huacai > > > > > /* Space for cache colour alignment */ > > if (cpu_has_dc_aliases) > > > > -- > > 2.49.0 > > > >
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index b630604c577f9ff3f2493b0f254363e499c8318c..66343cb6c1737c4217ddd8a2c3ca244fac0ef8a5 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -690,9 +690,11 @@ unsigned long mips_stack_top(void) } /* Space for the VDSO, data page & GIC user page */ - top -= PAGE_ALIGN(current->thread.abi->vdso->size); - top -= PAGE_SIZE; - top -= mips_gic_present() ? PAGE_SIZE : 0; + if (current->thread.abi) { + top -= PAGE_ALIGN(current->thread.abi->vdso->size); + top -= PAGE_SIZE; + top -= mips_gic_present() ? PAGE_SIZE : 0; + } /* Space for cache colour alignment */ if (cpu_has_dc_aliases)
Not all tasks have an ABI associated or vDSO mapped, for example kthreads never do. If such a task ever ends up calling stack_top(), it will derefence the NULL vdso pointer and crash. This can for example happen when using kunit: mips_stack_top+0x28/0xc0 arch_pick_mmap_layout+0x190/0x220 kunit_vm_mmap_init+0xf8/0x138 __kunit_add_resource+0x40/0xa8 kunit_vm_mmap+0x88/0xd8 usercopy_test_init+0xb8/0x240 kunit_try_run_case+0x5c/0x1a8 kunit_generic_run_threadfn_adapter+0x28/0x50 kthread+0x118/0x240 ret_from_kernel_thread+0x14/0x1c Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> --- arch/mips/kernel/process.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)