All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Safonov <dsafonov@virtuozzo.com>
To: linux-kernel@vger.kernel.org
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>,
	0x7f454c46@gmail.com, Cyrill Gorcunov <gorcunov@openvz.org>,
	Andy Lutomirski <luto@kernel.org>,
	oleg@redhat.com, Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	linux-mm@kvack.org, x86@kernel.org
Subject: [PATCH 2/2] x86/vdso: set vdso pointer only after success
Date: Thu, 27 Oct 2016 17:15:16 +0300	[thread overview]
Message-ID: <20161027141516.28447-3-dsafonov@virtuozzo.com> (raw)
In-Reply-To: <20161027141516.28447-1-dsafonov@virtuozzo.com>

Those pointers were initialized before call to _install_special_mapping
after the commit f7b6eb3fa072 ("x86: Set context.vdso before installing
the mapping"). This is not required anymore as special mappings have
their vma name and don't use arch_vma_name() after commit a62c34bd2a8a
("x86, mm: Improve _install_special_mapping and fix x86 vdso naming").
So, this way to init looks less entangled.
I even belive, we can remove null initializers:
- on failure load_elf_binary() will not start a new thread;
- arch_prctl will have the same pointers as before syscall.

Cc: 0x7f454c46@gmail.com
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: oleg@redhat.com
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
 arch/x86/entry/vdso/vma.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 23c881caabd1..e739002427ed 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -161,8 +161,6 @@ static int map_vdso(const struct vdso_image *image, unsigned long addr)
 	}
 
 	text_start = addr - image->sym_vvar_start;
-	current->mm->context.vdso = (void __user *)text_start;
-	current->mm->context.vdso_image = image;
 
 	/*
 	 * MAYWRITE to allow gdb to COW and set breakpoints
@@ -189,14 +187,12 @@ static int map_vdso(const struct vdso_image *image, unsigned long addr)
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		do_munmap(mm, text_start, image->size);
+	} else {
+		current->mm->context.vdso = (void __user *)text_start;
+		current->mm->context.vdso_image = image;
 	}
 
 up_fail:
-	if (ret) {
-		current->mm->context.vdso = NULL;
-		current->mm->context.vdso_image = NULL;
-	}
-
 	up_write(&mm->mmap_sem);
 	return ret;
 }
-- 
2.10.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Safonov <dsafonov@virtuozzo.com>
To: <linux-kernel@vger.kernel.org>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>, <0x7f454c46@gmail.com>,
	"Cyrill Gorcunov" <gorcunov@openvz.org>,
	Andy Lutomirski <luto@kernel.org>, <oleg@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	<linux-mm@kvack.org>, <x86@kernel.org>
Subject: [PATCH 2/2] x86/vdso: set vdso pointer only after success
Date: Thu, 27 Oct 2016 17:15:16 +0300	[thread overview]
Message-ID: <20161027141516.28447-3-dsafonov@virtuozzo.com> (raw)
In-Reply-To: <20161027141516.28447-1-dsafonov@virtuozzo.com>

Those pointers were initialized before call to _install_special_mapping
after the commit f7b6eb3fa072 ("x86: Set context.vdso before installing
the mapping"). This is not required anymore as special mappings have
their vma name and don't use arch_vma_name() after commit a62c34bd2a8a
("x86, mm: Improve _install_special_mapping and fix x86 vdso naming").
So, this way to init looks less entangled.
I even belive, we can remove null initializers:
- on failure load_elf_binary() will not start a new thread;
- arch_prctl will have the same pointers as before syscall.

Cc: 0x7f454c46@gmail.com
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: oleg@redhat.com
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
 arch/x86/entry/vdso/vma.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 23c881caabd1..e739002427ed 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -161,8 +161,6 @@ static int map_vdso(const struct vdso_image *image, unsigned long addr)
 	}
 
 	text_start = addr - image->sym_vvar_start;
-	current->mm->context.vdso = (void __user *)text_start;
-	current->mm->context.vdso_image = image;
 
 	/*
 	 * MAYWRITE to allow gdb to COW and set breakpoints
@@ -189,14 +187,12 @@ static int map_vdso(const struct vdso_image *image, unsigned long addr)
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		do_munmap(mm, text_start, image->size);
+	} else {
+		current->mm->context.vdso = (void __user *)text_start;
+		current->mm->context.vdso_image = image;
 	}
 
 up_fail:
-	if (ret) {
-		current->mm->context.vdso = NULL;
-		current->mm->context.vdso_image = NULL;
-	}
-
 	up_write(&mm->mmap_sem);
 	return ret;
 }
-- 
2.10.1

  parent reply	other threads:[~2016-10-27 14:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-27 14:15 [PATCH 0/2] x86/vdso: small fixups for map_vdso Dmitry Safonov
2016-10-27 14:15 ` Dmitry Safonov
2016-10-27 14:15 ` [PATCH 1/2] x86/prctl/uapi: remove ifdef for CHECKPOINT_RESTORE Dmitry Safonov
2016-10-27 14:15   ` Dmitry Safonov
2016-10-28  6:48   ` [tip:x86/asm] x86/prctl/uapi: Remove #ifdef " tip-bot for Dmitry Safonov
2016-10-27 14:15 ` Dmitry Safonov [this message]
2016-10-27 14:15   ` [PATCH 2/2] x86/vdso: set vdso pointer only after success Dmitry Safonov
2016-10-27 22:25   ` Andy Lutomirski
2016-10-27 22:25     ` Andy Lutomirski
2016-10-28  6:49   ` [tip:x86/asm] x86/vdso: Set vDSO " tip-bot for Dmitry Safonov

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=20161027141516.28447-3-dsafonov@virtuozzo.com \
    --to=dsafonov@virtuozzo.com \
    --cc=0x7f454c46@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=gorcunov@openvz.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --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.