On Tue, 15 Apr 2025 at 15:10, 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 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.weissschuh@linutronix.de

This looks good to me -- we've hit what's effectively the same issue on other architectures with this test, with similar resolutions.

Reviewed-by: David Gow davidgow@google.com

I'm happy to have this (and the following path) go in via the KUnit tree, but if you'd rather have it go via MIPS, that's fine, too. (The qemu_configs won't generate any merge conflicts.)

-- David

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;
  

•           /* 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

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.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 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.weissschuh@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

On Tue, 15 Apr 2025 at 15:10, Thomas Weißschuh thomas.weissschuh@linutronix.de wrote:

Add basic support to run various MIPS variants via kunit_tool using the virtualized malta platform.

Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de

Seems to work fine here. Thanks very much!

Reviewed-by: David Gow davidgow@google.com

Cheers, -- David

tools/testing/kunit/qemu_configs/mips.py | 18 ++++++++++++++++++ tools/testing/kunit/qemu_configs/mips64.py | 19 +++++++++++++++++++ tools/testing/kunit/qemu_configs/mips64el.py | 19 +++++++++++++++++++ tools/testing/kunit/qemu_configs/mipsel.py | 18 ++++++++++++++++++ 4 files changed, 74 insertions(+)

diff --git a/tools/testing/kunit/qemu_configs/mips.py b/tools/testing/kunit/qemu_configs/mips.py new file mode 100644 index 0000000000000000000000000000000000000000..8899ac157b30bd2ee847eacd5b90fe6ad4e5fb04 --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0

+from ..qemu_config import QemuArchParams

+QEMU_ARCH = QemuArchParams(linux_arch='mips',

•                       kconfig='''
  

+CONFIG_32BIT=y +CONFIG_CPU_BIG_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''',

•                       qemu_arch='mips',
  

•                       kernel_path='vmlinuz',
  

•                       kernel_command_line='console=ttyS0',
  

•                       extra_qemu_params=['-M', 'malta'])
  

diff --git a/tools/testing/kunit/qemu_configs/mips64.py b/tools/testing/kunit/qemu_configs/mips64.py new file mode 100644 index 0000000000000000000000000000000000000000..1478aed05b94da4914f34c6a8affdcfe34eb88ea --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips64.py @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0

+from ..qemu_config import QemuArchParams

+QEMU_ARCH = QemuArchParams(linux_arch='mips',

•                       kconfig='''
  

+CONFIG_CPU_MIPS64_R2=y +CONFIG_64BIT=y +CONFIG_CPU_BIG_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''',

•                       qemu_arch='mips64',
  

•                       kernel_path='vmlinuz',
  

•                       kernel_command_line='console=ttyS0',
  

•                       extra_qemu_params=['-M', 'malta', '-cpu', '5KEc'])
  

diff --git a/tools/testing/kunit/qemu_configs/mips64el.py b/tools/testing/kunit/qemu_configs/mips64el.py new file mode 100644 index 0000000000000000000000000000000000000000..300c711d7a82500b2ebcb4cf1467b6f72b5c17aa --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mips64el.py @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0

+from ..qemu_config import QemuArchParams

+QEMU_ARCH = QemuArchParams(linux_arch='mips',

•                       kconfig='''
  

+CONFIG_CPU_MIPS64_R2=y +CONFIG_64BIT=y +CONFIG_CPU_LITTLE_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''',

•                       qemu_arch='mips64el',
  

•                       kernel_path='vmlinuz',
  

•                       kernel_command_line='console=ttyS0',
  

•                       extra_qemu_params=['-M', 'malta', '-cpu', '5KEc'])
  

diff --git a/tools/testing/kunit/qemu_configs/mipsel.py b/tools/testing/kunit/qemu_configs/mipsel.py new file mode 100644 index 0000000000000000000000000000000000000000..3d3543315b45776d0e77fb5c00c8c0a89eafdffd --- /dev/null +++ b/tools/testing/kunit/qemu_configs/mipsel.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: GPL-2.0

+from ..qemu_config import QemuArchParams

+QEMU_ARCH = QemuArchParams(linux_arch='mips',

•                       kconfig='''
  

+CONFIG_32BIT=y +CONFIG_CPU_LITTLE_ENDIAN=y +CONFIG_MIPS_MALTA=y +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +CONFIG_POWER_RESET=y +CONFIG_POWER_RESET_SYSCON=y +''',

•                       qemu_arch='mipsel',
  

•                       kernel_path='vmlinuz',
  

•                       kernel_command_line='console=ttyS0',
  

•                       extra_qemu_params=['-M', 'malta'])
  
  

-- 2.49.0