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;
+	}
 
 	/* Space for cache colour alignment */
 	if (cpu_has_dc_aliases)

-- 
2.49.0

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