linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] kunit: qemu_configs: Add MIPS configurations
@ 2025-04-15  7:10 Thomas Weißschuh
  2025-04-15  7:10 ` [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Thomas Weißschuh
  2025-04-15  7:10 ` [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
  0 siblings, 2 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2025-04-15  7:10 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Brendan Higgins, David Gow, Rae Moar,
	Huacai Chen
  Cc: linux-mips, linux-kernel, linux-kselftest, kunit-dev,
	Thomas Weißschuh

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

Some of the cs_dsp unittests are broken. They are being disabled by default in
the series "Fix up building KUnit tests for Cirrus Logic modules" [0].

[0] https://lore.kernel.org/lkml/20250411123608.1676462-1-rf@opensource.cirrus.com/

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Changes in v3:
- Also skip VDSO_RANDOMIZE_SIZE adjustment for kthreads
- Link to v2: https://lore.kernel.org/r/20250414-kunit-mips-v2-0-4cf01e1a29e6@linutronix.de

Changes in v2:
- Fix usercopy kunit test by handling ABI-less tasks in stack_top()
- Drop change to mm initialization.
  The broken test is not built by default anymore.
- Link to v1: https://lore.kernel.org/r/20250212-kunit-mips-v1-0-eb49c9d76615@linutronix.de

---
Thomas Weißschuh (2):
      MIPS: Don't crash in stack_top() for tasks without ABI or vDSO
      kunit: qemu_configs: Add MIPS configurations

 arch/mips/kernel/process.c                   | 16 +++++++++-------
 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 ++++++++++++++++++
 5 files changed, 83 insertions(+), 7 deletions(-)
---
base-commit: 0466dc03fa779373afb807ce7496c404d98ace4b
change-id: 20241014-kunit-mips-e4fe1c265ed7

Best regards,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO
  2025-04-15  7:10 [PATCH v3 0/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
@ 2025-04-15  7:10 ` Thomas Weißschuh
  2025-04-15  8:26   ` David Gow
  2025-04-15  8:47   ` Huacai Chen
  2025-04-15  7:10 ` [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
  1 sibling, 2 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2025-04-15  7:10 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Brendan Higgins, David Gow, Rae Moar,
	Huacai Chen
  Cc: linux-mips, linux-kernel, linux-kselftest, kunit-dev,
	Thomas Weißschuh

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;
+
+		/* 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations
  2025-04-15  7:10 [PATCH v3 0/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
  2025-04-15  7:10 ` [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Thomas Weißschuh
@ 2025-04-15  7:10 ` Thomas Weißschuh
  2025-04-15  8:26   ` David Gow
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Weißschuh @ 2025-04-15  7:10 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Brendan Higgins, David Gow, Rae Moar,
	Huacai Chen
  Cc: linux-mips, linux-kernel, linux-kselftest, kunit-dev,
	Thomas Weißschuh

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO
  2025-04-15  7:10 ` [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Thomas Weißschuh
@ 2025-04-15  8:26   ` David Gow
  2025-04-15  8:47   ` Huacai Chen
  1 sibling, 0 replies; 7+ messages in thread
From: David Gow @ 2025-04-15  8:26 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Thomas Bogendoerfer, Brendan Higgins, Rae Moar, Huacai Chen,
	linux-mips, linux-kernel, linux-kselftest, kunit-dev

[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]

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
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations
  2025-04-15  7:10 ` [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
@ 2025-04-15  8:26   ` David Gow
  0 siblings, 0 replies; 7+ messages in thread
From: David Gow @ 2025-04-15  8:26 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Thomas Bogendoerfer, Brendan Higgins, Rae Moar, Huacai Chen,
	linux-mips, linux-kernel, linux-kselftest, kunit-dev

[-- Attachment #1: Type: text/plain, Size: 4570 bytes --]

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
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO
  2025-04-15  7:10 ` [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Thomas Weißschuh
  2025-04-15  8:26   ` David Gow
@ 2025-04-15  8:47   ` Huacai Chen
  2025-04-15  8:57     ` Thomas Weißschuh
  1 sibling, 1 reply; 7+ messages in thread
From: Huacai Chen @ 2025-04-15  8:47 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Thomas Bogendoerfer, Brendan Higgins, David Gow, Rae Moar,
	linux-mips, linux-kernel, linux-kselftest, kunit-dev

Hi, Thomas,

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.

Huacai

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO
  2025-04-15  8:47   ` Huacai Chen
@ 2025-04-15  8:57     ` Thomas Weißschuh
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Weißschuh @ 2025-04-15  8:57 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Thomas Bogendoerfer, Brendan Higgins, David Gow, Rae Moar,
	linux-mips, linux-kernel, linux-kselftest, kunit-dev

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-15  8:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-15  7:10 [PATCH v3 0/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
2025-04-15  7:10 ` [PATCH v3 1/2] MIPS: Don't crash in stack_top() for tasks without ABI or vDSO Thomas Weißschuh
2025-04-15  8:26   ` David Gow
2025-04-15  8:47   ` Huacai Chen
2025-04-15  8:57     ` Thomas Weißschuh
2025-04-15  7:10 ` [PATCH v3 2/2] kunit: qemu_configs: Add MIPS configurations Thomas Weißschuh
2025-04-15  8:26   ` David Gow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).