Not all tasks have a 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:
[<9000000000203874>] stack_top+0x58/0xa8
[<90000000002956cc>] arch_pick_mmap_layout+0x164/0x220
[<90000000003c284c>] kunit_vm_mmap_init+0x108/0x12c
[<90000000003c1fbc>] __kunit_add_resource+0x38/0x8c
[<90000000003c2704>] kunit_vm_mmap+0x88/0xc8
[<9000000000410b14>] usercopy_test_init+0xbc/0x25c
[<90000000003c1db4>] kunit_try_run_case+0x5c/0x184
[<90000000003c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48
[<900000000022e4bc>] kthread+0xc8/0xd4
[<9000000000200ce8>] ret_from_kernel_thread+0xc/0xa4
Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
arch/loongarch/kernel/process.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -293,13 +293,15 @@ unsigned long stack_top(void)
{
unsigned long top = TASK_SIZE & PAGE_MASK;
- /* Space for the VDSO & data page */
- top -= PAGE_ALIGN(current->thread.vdso->size);
- top -= VVAR_SIZE;
-
- /* Space to randomize the VDSO base */
- if (current->flags & PF_RANDOMIZE)
- top -= VDSO_RANDOMIZE_SIZE;
+ if (current->thread.vdso) {
+ /* Space for the VDSO & data page */
+ top -= PAGE_ALIGN(current->thread.vdso->size);
+ top -= VVAR_SIZE;
+
+ /* Space to randomize the VDSO base */
+ if (current->flags & PF_RANDOMIZE)
+ top -= VDSO_RANDOMIZE_SIZE;
+ }
return top;
}
--
2.47.0
On Mon, 14 Oct 2024 at 19:36, Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> Not all tasks have a 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:
>
> [<9000000000203874>] stack_top+0x58/0xa8
> [<90000000002956cc>] arch_pick_mmap_layout+0x164/0x220
> [<90000000003c284c>] kunit_vm_mmap_init+0x108/0x12c
> [<90000000003c1fbc>] __kunit_add_resource+0x38/0x8c
> [<90000000003c2704>] kunit_vm_mmap+0x88/0xc8
> [<9000000000410b14>] usercopy_test_init+0xbc/0x25c
> [<90000000003c1db4>] kunit_try_run_case+0x5c/0x184
> [<90000000003c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48
> [<900000000022e4bc>] kthread+0xc8/0xd4
> [<9000000000200ce8>] ret_from_kernel_thread+0xc/0xa4
>
> Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
Thanks very much for fixing this: the usercopy tests and kunit_vm_mmap
stuff has been broken on quite a few architectures, so I really
appreciate this being fixed "day one" for KUnit support. :-)
We'll take the rest of the patches via the kunit/kselftest tree; I
agree that it makes more sense for this one to go in separately via
loongarch.
Thanks,
-- David
> arch/loongarch/kernel/process.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
> index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644
> --- a/arch/loongarch/kernel/process.c
> +++ b/arch/loongarch/kernel/process.c
> @@ -293,13 +293,15 @@ unsigned long stack_top(void)
> {
> unsigned long top = TASK_SIZE & PAGE_MASK;
>
> - /* Space for the VDSO & data page */
> - top -= PAGE_ALIGN(current->thread.vdso->size);
> - top -= VVAR_SIZE;
> -
> - /* Space to randomize the VDSO base */
> - if (current->flags & PF_RANDOMIZE)
> - top -= VDSO_RANDOMIZE_SIZE;
> + if (current->thread.vdso) {
> + /* Space for the VDSO & data page */
> + top -= PAGE_ALIGN(current->thread.vdso->size);
> + top -= VVAR_SIZE;
> +
> + /* Space to randomize the VDSO base */
> + if (current->flags & PF_RANDOMIZE)
> + top -= VDSO_RANDOMIZE_SIZE;
> + }
>
> return top;
> }
>
> --
> 2.47.0
>
Hi, Thomas,
I can take this patch to the loongarch tree, but I think others should
get upstream via kselftests tree?
Huacai
On Mon, Oct 14, 2024 at 7:36 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> Not all tasks have a 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:
>
> [<9000000000203874>] stack_top+0x58/0xa8
> [<90000000002956cc>] arch_pick_mmap_layout+0x164/0x220
> [<90000000003c284c>] kunit_vm_mmap_init+0x108/0x12c
> [<90000000003c1fbc>] __kunit_add_resource+0x38/0x8c
> [<90000000003c2704>] kunit_vm_mmap+0x88/0xc8
> [<9000000000410b14>] usercopy_test_init+0xbc/0x25c
> [<90000000003c1db4>] kunit_try_run_case+0x5c/0x184
> [<90000000003c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48
> [<900000000022e4bc>] kthread+0xc8/0xd4
> [<9000000000200ce8>] ret_from_kernel_thread+0xc/0xa4
>
> Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
> arch/loongarch/kernel/process.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
> index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644
> --- a/arch/loongarch/kernel/process.c
> +++ b/arch/loongarch/kernel/process.c
> @@ -293,13 +293,15 @@ unsigned long stack_top(void)
> {
> unsigned long top = TASK_SIZE & PAGE_MASK;
>
> - /* Space for the VDSO & data page */
> - top -= PAGE_ALIGN(current->thread.vdso->size);
> - top -= VVAR_SIZE;
> -
> - /* Space to randomize the VDSO base */
> - if (current->flags & PF_RANDOMIZE)
> - top -= VDSO_RANDOMIZE_SIZE;
> + if (current->thread.vdso) {
> + /* Space for the VDSO & data page */
> + top -= PAGE_ALIGN(current->thread.vdso->size);
> + top -= VVAR_SIZE;
> +
> + /* Space to randomize the VDSO base */
> + if (current->flags & PF_RANDOMIZE)
> + top -= VDSO_RANDOMIZE_SIZE;
> + }
>
> return top;
> }
>
> --
> 2.47.0
>
Hi Huacai,
On Tue, Oct 15, 2024 at 10:15:39AM +0800, Huacai Chen wrote:
> I can take this patch to the loongarch tree, but I think others should
> get upstream via kselftests tree?
Yes, sounds good.
Could you take a look at patches 2 and 4, too?
Thanks,
Thomas
> On Mon, Oct 14, 2024 at 7:36 PM Thomas Weißschuh
> <thomas.weissschuh@linutronix.de> wrote:
> >
> > Not all tasks have a 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:
> >
> > [<9000000000203874>] stack_top+0x58/0xa8
> > [<90000000002956cc>] arch_pick_mmap_layout+0x164/0x220
> > [<90000000003c284c>] kunit_vm_mmap_init+0x108/0x12c
> > [<90000000003c1fbc>] __kunit_add_resource+0x38/0x8c
> > [<90000000003c2704>] kunit_vm_mmap+0x88/0xc8
> > [<9000000000410b14>] usercopy_test_init+0xbc/0x25c
> > [<90000000003c1db4>] kunit_try_run_case+0x5c/0x184
> > [<90000000003c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48
> > [<900000000022e4bc>] kthread+0xc8/0xd4
> > [<9000000000200ce8>] ret_from_kernel_thread+0xc/0xa4
> >
> > Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> > arch/loongarch/kernel/process.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
> > index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644
> > --- a/arch/loongarch/kernel/process.c
> > +++ b/arch/loongarch/kernel/process.c
> > @@ -293,13 +293,15 @@ unsigned long stack_top(void)
> > {
> > unsigned long top = TASK_SIZE & PAGE_MASK;
> >
> > - /* Space for the VDSO & data page */
> > - top -= PAGE_ALIGN(current->thread.vdso->size);
> > - top -= VVAR_SIZE;
> > -
> > - /* Space to randomize the VDSO base */
> > - if (current->flags & PF_RANDOMIZE)
> > - top -= VDSO_RANDOMIZE_SIZE;
> > + if (current->thread.vdso) {
> > + /* Space for the VDSO & data page */
> > + top -= PAGE_ALIGN(current->thread.vdso->size);
> > + top -= VVAR_SIZE;
> > +
> > + /* Space to randomize the VDSO base */
> > + if (current->flags & PF_RANDOMIZE)
> > + top -= VDSO_RANDOMIZE_SIZE;
> > + }
> >
> > return top;
> > }
> >
> > --
> > 2.47.0
> >
On Tue, Oct 15, 2024 at 2:14 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> Hi Huacai,
>
> On Tue, Oct 15, 2024 at 10:15:39AM +0800, Huacai Chen wrote:
> > I can take this patch to the loongarch tree, but I think others should
> > get upstream via kselftests tree?
>
> Yes, sounds good.
> Could you take a look at patches 2 and 4, too?
Bibo is more familiar with those, so +CC Bibo.
This one is queued in the loongarch tree. Thanks.
Huacai
>
> Thanks,
> Thomas
>
> > On Mon, Oct 14, 2024 at 7:36 PM Thomas Weißschuh
> > <thomas.weissschuh@linutronix.de> wrote:
> > >
> > > Not all tasks have a 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:
> > >
> > > [<9000000000203874>] stack_top+0x58/0xa8
> > > [<90000000002956cc>] arch_pick_mmap_layout+0x164/0x220
> > > [<90000000003c284c>] kunit_vm_mmap_init+0x108/0x12c
> > > [<90000000003c1fbc>] __kunit_add_resource+0x38/0x8c
> > > [<90000000003c2704>] kunit_vm_mmap+0x88/0xc8
> > > [<9000000000410b14>] usercopy_test_init+0xbc/0x25c
> > > [<90000000003c1db4>] kunit_try_run_case+0x5c/0x184
> > > [<90000000003c3d54>] kunit_generic_run_threadfn_adapter+0x24/0x48
> > > [<900000000022e4bc>] kthread+0xc8/0xd4
> > > [<9000000000200ce8>] ret_from_kernel_thread+0xc/0xa4
> > >
> > > Fixes: 803b0fc5c3f2 ("LoongArch: Add process management")
> > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > > ---
> > > arch/loongarch/kernel/process.c | 16 +++++++++-------
> > > 1 file changed, 9 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
> > > index f2ff8b5d591e4fd638109d2c98d75543c01a112c..6e58f65455c7ca3eae2e88ed852c8655a6701e5c 100644
> > > --- a/arch/loongarch/kernel/process.c
> > > +++ b/arch/loongarch/kernel/process.c
> > > @@ -293,13 +293,15 @@ unsigned long stack_top(void)
> > > {
> > > unsigned long top = TASK_SIZE & PAGE_MASK;
> > >
> > > - /* Space for the VDSO & data page */
> > > - top -= PAGE_ALIGN(current->thread.vdso->size);
> > > - top -= VVAR_SIZE;
> > > -
> > > - /* Space to randomize the VDSO base */
> > > - if (current->flags & PF_RANDOMIZE)
> > > - top -= VDSO_RANDOMIZE_SIZE;
> > > + if (current->thread.vdso) {
> > > + /* Space for the VDSO & data page */
> > > + top -= PAGE_ALIGN(current->thread.vdso->size);
> > > + top -= VVAR_SIZE;
> > > +
> > > + /* Space to randomize the VDSO base */
> > > + if (current->flags & PF_RANDOMIZE)
> > > + top -= VDSO_RANDOMIZE_SIZE;
> > > + }
> > >
> > > return top;
> > > }
> > >
> > > --
> > > 2.47.0
> > >
© 2016 - 2024 Red Hat, Inc.