All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Safonov <dima@arista.com>
To: linux-kernel@vger.kernel.org
Cc: Dmitry Safonov <0x7f454c46@gmail.com>,
	Dmitry Safonov <dima@arista.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Borislav Petkov <bp@alien8.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Guo Ren <guoren@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@redhat.com>, Oleg Nesterov <oleg@redhat.com>,
	Russell King <linux@armlinux.org.uk>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Will Deacon <will@kernel.org>,
	x86@kernel.org
Subject: [PATCH v3 07/23] vdso: Set mm->context.vdso only on success of _install_special_mapping()
Date: Fri, 11 Jun 2021 19:02:26 +0100	[thread overview]
Message-ID: <20210611180242.711399-8-dima@arista.com> (raw)
In-Reply-To: <20210611180242.711399-1-dima@arista.com>

Old pattern was:
1. set mm->context.vdso = addr;
2. try install_special_mapping()
3. on failure set mm->context.vdso = 0

The reason behind the old pattern was to make arch_vma_name() work
in perf_event_mmap_event().  These days using _install_special_mapping()
instead of install_special_mapping() makes old pattern obsolete:
:		if (vma->vm_ops && vma->vm_ops->name) {
:			name = (char *) vma->vm_ops->name(vma);
:			if (name)
:				goto cpy_name;

Setting mm->context.vdso = 0 also makes little sense: mm_alloc()
zero-fills new mm_struct. And for double-safety if
arch_setup_additional_pages() fails, bprm_execve() makes sure that the
half-initialized process doesn't make it way to userspace by
:		force_sigsegv(SIGSEGV);

Let's cleanup code: set mm->context.vdso only on success, assuming that
any new mm_struct is clean.

Some platforms do_munmap() vvar if vdso mapping failed, but it's really
necessary only on x86 where vdso/vvar pair can be mapped by userspace
(see prctl_map_vdso()). On other platforms vdso/vvar is only pre-mapped
by ELF loader, which as described above will make sure to not let any
half-baked process out. I've left do_unmap() on !x86 in case prctl()
will be supported.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/arm/kernel/vdso.c     |  2 --
 arch/arm64/kernel/vdso.c   | 16 +++++-----------
 arch/nds32/kernel/vdso.c   |  5 +----
 arch/powerpc/kernel/vdso.c |  9 ++-------
 arch/sparc/vdso/vma.c      |  7 ++-----
 5 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index 3408269d19c7..015eff0a6e93 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -238,8 +238,6 @@ void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
 	struct vm_area_struct *vma;
 	unsigned long len;
 
-	mm->context.vdso = 0;
-
 	if (vdso_text_pagelist == NULL)
 		return;
 
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index a8bf72320ad0..1bc8adefa293 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -227,34 +227,28 @@ static int __setup_additional_pages(enum vdso_abi abi,
 	vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
 
 	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
-	if (IS_ERR_VALUE(vdso_base)) {
-		ret = ERR_PTR(vdso_base);
-		goto up_fail;
-	}
+	if (IS_ERR_VALUE(vdso_base))
+		return vdso_base;
 
 	ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
 				       VM_READ|VM_MAYREAD|VM_PFNMAP,
 				       vdso_info[abi].dm);
 	if (IS_ERR(ret))
-		goto up_fail;
+		return PTR_ERR(ret);
 
 	if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && system_supports_bti())
 		gp_flags = VM_ARM64_BTI;
 
 	vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
-	mm->context.vdso = (void *)vdso_base;
 	ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
 				       VM_READ|VM_EXEC|gp_flags|
 				       VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
 				       vdso_info[abi].cm);
 	if (IS_ERR(ret))
-		goto up_fail;
+		return PTR_ERR(ret);
 
+	mm->context.vdso = (void *)vdso_base;
 	return 0;
-
-up_fail:
-	mm->context.vdso = NULL;
-	return PTR_ERR(ret);
 }
 
 #ifdef CONFIG_COMPAT
diff --git a/arch/nds32/kernel/vdso.c b/arch/nds32/kernel/vdso.c
index e16009a07971..2d1d51a0fc64 100644
--- a/arch/nds32/kernel/vdso.c
+++ b/arch/nds32/kernel/vdso.c
@@ -175,7 +175,6 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 
 	/*Map vdso to user space */
 	vdso_base += PAGE_SIZE;
-	mm->context.vdso = (void *)vdso_base;
 	vma = _install_special_mapping(mm, vdso_base, vdso_text_len,
 				       VM_READ | VM_EXEC |
 				       VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
@@ -185,11 +184,9 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 		goto up_fail;
 	}
 
-	mmap_write_unlock(mm);
-	return 0;
+	mm->context.vdso = (void *)vdso_base;
 
 up_fail:
-	mm->context.vdso = NULL;
 	mmap_write_unlock(mm);
 	return ret;
 }
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 717f2c9a7573..76e898b56002 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -229,13 +229,6 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
 	/* Add required alignment. */
 	vdso_base = ALIGN(vdso_base, VDSO_ALIGNMENT);
 
-	/*
-	 * Put vDSO base into mm struct. We need to do this before calling
-	 * install_special_mapping or the perf counter mmap tracking code
-	 * will fail to recognise it as a vDSO.
-	 */
-	mm->context.vdso = (void __user *)vdso_base + vvar_size;
-
 	vma = _install_special_mapping(mm, vdso_base, vvar_size,
 				       VM_READ | VM_MAYREAD | VM_IO |
 				       VM_DONTDUMP | VM_PFNMAP, &vvar_spec);
@@ -257,6 +250,8 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
 				       VM_MAYWRITE | VM_MAYEXEC, vdso_spec);
 	if (IS_ERR(vma))
 		do_munmap(mm, vdso_base, vvar_size, NULL);
+	else
+		mm->context.vdso = (void __user *)vdso_base + vvar_size;
 
 	return PTR_ERR_OR_ZERO(vma);
 }
diff --git a/arch/sparc/vdso/vma.c b/arch/sparc/vdso/vma.c
index cc19e09b0fa1..d8a344f6c914 100644
--- a/arch/sparc/vdso/vma.c
+++ b/arch/sparc/vdso/vma.c
@@ -390,7 +390,6 @@ static int map_vdso(const struct vdso_image *image,
 	}
 
 	text_start = addr - image->sym_vvar_start;
-	current->mm->context.vdso = (void __user *)text_start;
 
 	/*
 	 * MAYWRITE to allow gdb to COW and set breakpoints
@@ -412,16 +411,14 @@ static int map_vdso(const struct vdso_image *image,
 				       -image->sym_vvar_start,
 				       VM_READ|VM_MAYREAD,
 				       &vvar_mapping);
-
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		do_munmap(mm, text_start, image->size, NULL);
+	} else {
+		current->mm->context.vdso = (void __user *)text_start;
 	}
 
 up_fail:
-	if (ret)
-		current->mm->context.vdso = NULL;
-
 	mmap_write_unlock(mm);
 	return ret;
 }
-- 
2.31.1


  parent reply	other threads:[~2021-06-11 18:04 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 18:02 [PATCH v3 00/23] Add generic vdso_base tracking Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 01/23] x86/elf: Check in_x32_syscall() in compat_arch_setup_additional_pages() Dmitry Safonov
2021-06-19 20:41   ` Thomas Gleixner
2021-06-21 20:59     ` Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 02/23] elf: Move arch_setup_additional_pages() to generic elf.h Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 03/23] arm/elf: Remove needless ifdef CONFIG_MMU Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 04/23] arm64: Use in_compat_task() in arch_setup_additional_pages() Dmitry Safonov
2021-06-11 18:02   ` Dmitry Safonov
2021-06-15 10:21   ` Will Deacon
2021-06-15 10:21     ` Will Deacon
2021-06-11 18:02 ` [PATCH v3 05/23] x86: Remove compat_arch_setup_additional_pages() Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 06/23] elf: " Dmitry Safonov
2021-06-11 18:02 ` Dmitry Safonov [this message]
2021-06-11 18:02 ` [PATCH v3 08/23] elf/vdso: Modify arch_setup_additional_pages() parameters Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 09/23] elf: Use sysinfo_ehdr in ARCH_DLINFO() Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 10/23] arm/vdso: Remove vdso pointer from mm->context Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 11/23] s390/vdso: Remove vdso_base " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 12/23] sparc/vdso: Remove vdso " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 13/23] mm/mmap: Make vm_special_mapping::mremap return void Dmitry Safonov
2021-06-17  7:20   ` Christophe Leroy
2021-06-21 21:12     ` Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 14/23] x86/signal: Land on &frame->retcode when vdso isn't mapped Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 15/23] x86/signal: Check if vdso_image_32 is mapped before trying to land on it Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 16/23] mm: Add vdso_base in mm_struct Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 17/23] x86/vdso: Migrate to generic vdso_base Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 18/23] arm/vdso: " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 19/23] arm64/vdso: Migrate compat signals " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 20/23] arm64/vdso: Migrate native " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 21/23] mips/vdso: Migrate " Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 22/23] powerpc/vdso: Migrate native signals " Dmitry Safonov
2021-06-15 12:52   ` Michael Ellerman
2021-06-17  6:30   ` Christophe Leroy
2021-06-17  6:36   ` Christophe Leroy
2021-06-17  7:34     ` Christophe Leroy
2021-06-21 21:22       ` Dmitry Safonov
2021-06-11 18:02 ` [PATCH v3 23/23] x86/vdso/selftest: Add a test for unmapping vDSO Dmitry Safonov
2021-06-11 18:21   ` Shuah Khan
2021-06-11 18:37     ` Dmitry Safonov
2021-06-11 18:43       ` Shuah Khan
2021-06-17  9:13 ` [PATCH v3 00/23] Add generic vdso_base tracking Christophe Leroy
2021-06-21 21:57   ` Dmitry Safonov
2022-03-09 15:41 ` Christophe Leroy
2022-03-10 21:17   ` Dmitry Safonov
2022-08-19  9:17     ` Christophe Leroy
2022-08-23 19:13       ` Dmitry Safonov
2023-10-11 10:28         ` Christophe Leroy
2023-10-11 23:20 ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210611180242.711399-8-dima@arista.com \
    --to=dima@arista.com \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=guoren@kernel.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tsbogend@alpha.franken.de \
    --cc=vincenzo.frascino@arm.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.