Hi Huacai,
On Tue, Apr 15, 2025 at 04:47:31PM +0800, Huacai Chen wrote:
> On Tue, Apr 15, 2025 at 3:10 PM Thomas Weißschuh
> <thomas.weisssc...@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 ABI 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
> >
> > Only dereference the ABI point if it is set.
> >
> > Also move the randomization adjustment into the same conditional.
> >
> > Signed-off-by: Thomas Weißschuh <thomas.weisssc...@linutronix.de>
> > ---
> > arch/mips/kernel/process.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
> > index
> > b630604c577f9ff3f2493b0f254363e499c8318c..02aa6a04a21da437909eeac4f149155cc298f5b5
> > 100644
> > --- a/arch/mips/kernel/process.c
> > +++ b/arch/mips/kernel/process.c
> > @@ -690,18 +690,20 @@ 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'm not sure, but maybe this line has nothing to do with VDSO.
The GIC page is also used by vDSO.
It is mapped into the process together with the regular vDSO pages in
arch_setup_additional_pages().
They should only ever be there together.
Thomas
> > +
> > + /* Space to randomize the VDSO base */
> > + if (current->flags & PF_RANDOMIZE)
> > + top -= VDSO_RANDOMIZE_SIZE;
> > + }
> >
> > /* Space for cache colour alignment */
> > if (cpu_has_dc_aliases)
> > top -= shm_align_mask + 1;
> >
> > - /* Space to randomize the VDSO base */
> > - if (current->flags & PF_RANDOMIZE)
> > - top -= VDSO_RANDOMIZE_SIZE;
> > -
> > return top;
> > }
> >
> >
> > --
> > 2.49.0
> >
