linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
@ 2016-05-10 13:29 Dmitry Safonov
  2016-05-10 13:29 ` [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test Dmitry Safonov
  2016-05-16  9:38 ` [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
  0 siblings, 2 replies; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-10 13:29 UTC (permalink / raw)
  To: linux-kernel, mingo
  Cc: luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46, Dmitry Safonov

Add possibility for userspace 32-bit applications to move
vdso mapping. Previously, when userspace app called
mremap for vdso, in return path it would land on previous
address of vdso page, resulting in segmentation violation.
Now it lands fine and returns to userspace with remapped vdso.
This will also fix context.vdso pointer for 64-bit, which does not
affect the user of vdso after mremap by now, but this may change.

As suggested by Andy, return EINVAL for mremap that splits vdso image.

Renamed and moved text_mapping structure declaration inside
map_vdso, as it used only there and now it complement
vvar_mapping variable.

There is still problem for remapping vdso in glibc applications:
linker relocates addresses for syscalls on vdso page, so
you need to relink with the new addresses. Or the next syscall
through glibc may fail:
  Program received signal SIGSEGV, Segmentation fault.
  #0  0xf7fd9b80 in __kernel_vsyscall ()
  #1  0xf7ec8238 in _exit () from /usr/lib32/libc.so.6

Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
---
v8: add WARN_ON_ONCE on current->mm != new_vma->vm_mm
v7: build fix
v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function
    with ifdefs around
v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are
    the same, also check not only in_ia32_syscall() but image == &vdso_image_32
v4: drop __maybe_unused & use image from mm->context instead vdso_image_32
v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap;
    used is_ia32_task instead of ifdefs 
v2: added __maybe_unused for pt_regs in vdso_mremap

 arch/x86/entry/vdso/vma.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/mm_types.h  |  3 +++
 mm/mmap.c                 | 10 ++++++++++
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 10f704584922..0625b66e5648 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -12,6 +12,7 @@
 #include <linux/random.h>
 #include <linux/elf.h>
 #include <linux/cpu.h>
+#include <linux/ptrace.h>
 #include <asm/pvclock.h>
 #include <asm/vgtod.h>
 #include <asm/proto.h>
@@ -98,10 +99,40 @@ static int vdso_fault(const struct vm_special_mapping *sm,
 	return 0;
 }
 
-static const struct vm_special_mapping text_mapping = {
-	.name = "[vdso]",
-	.fault = vdso_fault,
-};
+static void vdso_fix_landing(const struct vdso_image *image,
+		struct vm_area_struct *new_vma)
+{
+#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
+	if (in_ia32_syscall() && image == &vdso_image_32) {
+		struct pt_regs *regs = current_pt_regs();
+		unsigned long vdso_land = image->sym_int80_landing_pad;
+		unsigned long old_land_addr = vdso_land +
+			(unsigned long)current->mm->context.vdso;
+
+		/* Fixing userspace landing - look at do_fast_syscall_32 */
+		if (regs->ip == old_land_addr)
+			regs->ip = new_vma->vm_start + vdso_land;
+	}
+#endif
+}
+
+static int vdso_mremap(const struct vm_special_mapping *sm,
+		struct vm_area_struct *new_vma)
+{
+	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
+	const struct vdso_image *image = current->mm->context.vdso_image;
+
+	if (image->size != new_size)
+		return -EINVAL;
+
+	if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
+		return -EFAULT;
+
+	vdso_fix_landing(image, new_vma);
+	current->mm->context.vdso = (void __user *)new_vma->vm_start;
+
+	return 0;
+}
 
 static int vvar_fault(const struct vm_special_mapping *sm,
 		      struct vm_area_struct *vma, struct vm_fault *vmf)
@@ -162,6 +193,12 @@ static int map_vdso(const struct vdso_image *image, bool calculate_addr)
 	struct vm_area_struct *vma;
 	unsigned long addr, text_start;
 	int ret = 0;
+
+	static const struct vm_special_mapping vdso_mapping = {
+		.name = "[vdso]",
+		.fault = vdso_fault,
+		.mremap = vdso_mremap,
+	};
 	static const struct vm_special_mapping vvar_mapping = {
 		.name = "[vvar]",
 		.fault = vvar_fault,
@@ -195,7 +232,7 @@ static int map_vdso(const struct vdso_image *image, bool calculate_addr)
 				       image->size,
 				       VM_READ|VM_EXEC|
 				       VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
-				       &text_mapping);
+				       &vdso_mapping);
 
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index c2d75b4fa86c..4d16ab9287af 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -586,6 +586,9 @@ struct vm_special_mapping {
 	int (*fault)(const struct vm_special_mapping *sm,
 		     struct vm_area_struct *vma,
 		     struct vm_fault *vmf);
+
+	int (*mremap)(const struct vm_special_mapping *sm,
+		     struct vm_area_struct *new_vma);
 };
 
 enum tlb_flush_reason {
diff --git a/mm/mmap.c b/mm/mmap.c
index bd2e1a533bc1..ba71658dd1a1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2930,9 +2930,19 @@ static const char *special_mapping_name(struct vm_area_struct *vma)
 	return ((struct vm_special_mapping *)vma->vm_private_data)->name;
 }
 
+static int special_mapping_mremap(struct vm_area_struct *new_vma)
+{
+	struct vm_special_mapping *sm = new_vma->vm_private_data;
+
+	if (sm->mremap)
+		return sm->mremap(sm, new_vma);
+	return 0;
+}
+
 static const struct vm_operations_struct special_mapping_vmops = {
 	.close = special_mapping_close,
 	.fault = special_mapping_fault,
+	.mremap = special_mapping_mremap,
 	.name = special_mapping_name,
 };
 
-- 
2.8.0

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

* [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test
  2016-05-10 13:29 [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
@ 2016-05-10 13:29 ` Dmitry Safonov
  2016-05-16 13:54   ` Ingo Molnar
  2016-05-16  9:38 ` [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-10 13:29 UTC (permalink / raw)
  To: linux-kernel, mingo
  Cc: luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46, Dmitry Safonov,
	Shuah Khan, linux-kselftest

Should print on success:
[root@localhost ~]# ./test_mremap_vdso_32
	AT_SYSINFO_EHDR is 0xf773f000
[NOTE]	Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
[OK]
Or segfault if landing was bad (before patches):
[root@localhost ~]# ./test_mremap_vdso_32
	AT_SYSINFO_EHDR is 0xf774f000
[NOTE]	Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
Segmentation fault (core dumped)

Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: linux-kselftest@vger.kernel.org
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
---
v8: run test for x86_64 too;
    removed fixed VDSO_SIZE - check EINVAL mremap return for partial remapping
v5: initial version

 tools/testing/selftests/x86/Makefile           |  2 +-
 tools/testing/selftests/x86/test_mremap_vdso.c | 99 ++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/test_mremap_vdso.c

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index b47ebd170690..ba865f2efcce 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -5,7 +5,7 @@ include ../lib.mk
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall \
-			check_initial_reg_state sigreturn ldt_gdt iopl
+			check_initial_reg_state sigreturn ldt_gdt iopl test_mremap_vdso
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/test_mremap_vdso.c b/tools/testing/selftests/x86/test_mremap_vdso.c
new file mode 100644
index 000000000000..831e2e0107d9
--- /dev/null
+++ b/tools/testing/selftests/x86/test_mremap_vdso.c
@@ -0,0 +1,99 @@
+/*
+ * 32-bit test to check vdso mremap.
+ *
+ * Copyright (c) 2016 Dmitry Safonov
+ * Suggested-by: Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+/*
+ * Can be built statically:
+ * gcc -Os -Wall -static -m32 test_mremap_vdso.c
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/mman.h>
+#include <sys/auxv.h>
+#include <sys/syscall.h>
+
+#define PAGE_SIZE	4096
+
+static int try_to_remap(void *vdso_addr, unsigned long size)
+{
+	void *dest_addr, *new_addr;
+
+	dest_addr = mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+	if (dest_addr == MAP_FAILED) {
+		printf("[WARN]\tmmap failed (%d): %m\n", errno);
+		return 0;
+	}
+
+	printf("[NOTE]\tMoving vDSO: [%p, %#lx] -> [%p, %#lx]\n",
+		vdso_addr, (unsigned long)vdso_addr + size,
+		dest_addr, (unsigned long)dest_addr + size);
+	fflush(stdout);
+
+	new_addr = mremap(vdso_addr, size, size,
+			MREMAP_FIXED|MREMAP_MAYMOVE, dest_addr);
+	if ((unsigned long)new_addr == (unsigned long)-1) {
+		munmap(dest_addr, size);
+		if (errno == EINVAL) {
+			printf("[NOTE]\tvDSO partial move failed, will try with bigger size\n");
+			return -1; /* retry with larger */
+		}
+		printf("[FAIL]\tmremap failed (%d): %m\n", errno);
+		return 1;
+	}
+
+	return 0;
+
+}
+
+int main(int argc, char **argv, char **envp)
+{
+	unsigned long auxval;
+	const char *ok_string = "[OK]\n";
+	int ret = -1;
+	unsigned long vdso_size = PAGE_SIZE;
+
+	auxval = getauxval(AT_SYSINFO_EHDR);
+	printf("\tAT_SYSINFO_EHDR is %#lx\n", auxval);
+	if (!auxval || auxval == -ENOENT) {
+		printf("[WARN]\tgetauxval failed\n");
+		return 0;
+	}
+
+	/* simpler than parsing ELF header */
+	while(ret < 0) {
+		ret = try_to_remap((void *)auxval, vdso_size);
+		vdso_size += PAGE_SIZE;
+	}
+
+	if (!ret)
+#if defined(__i386__)
+		asm volatile ("int $0x80" : :
+			"a" (__NR_write), "b" (STDOUT_FILENO),
+			"c" (ok_string), "d" (strlen(ok_string)));
+
+	asm volatile ("int $0x80" : : "a" (__NR_exit), "b" (!!ret));
+#else
+		asm volatile ("syscall" : :
+			"a" (__NR_write), "D" (STDOUT_FILENO),
+			"S" (ok_string), "d" (strlen(ok_string)));
+
+	asm volatile ("syscall" : : "a" (__NR_exit), "D" (!!ret));
+#endif
+
+	return 0;
+}
-- 
2.8.0

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

* Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
  2016-05-10 13:29 [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
  2016-05-10 13:29 ` [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test Dmitry Safonov
@ 2016-05-16  9:38 ` Dmitry Safonov
  2016-05-16 10:54   ` Ingo Molnar
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-16  9:38 UTC (permalink / raw)
  To: linux-kernel, mingo; +Cc: luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46

On 05/10/2016 04:29 PM, Dmitry Safonov wrote:
> Add possibility for userspace 32-bit applications to move
> vdso mapping. Previously, when userspace app called
> mremap for vdso, in return path it would land on previous
> address of vdso page, resulting in segmentation violation.
> Now it lands fine and returns to userspace with remapped vdso.
> This will also fix context.vdso pointer for 64-bit, which does not
> affect the user of vdso after mremap by now, but this may change.
>
> As suggested by Andy, return EINVAL for mremap that splits vdso image.
>
> Renamed and moved text_mapping structure declaration inside
> map_vdso, as it used only there and now it complement
> vvar_mapping variable.
>
> There is still problem for remapping vdso in glibc applications:
> linker relocates addresses for syscalls on vdso page, so
> you need to relink with the new addresses. Or the next syscall
> through glibc may fail:
>   Program received signal SIGSEGV, Segmentation fault.
>   #0  0xf7fd9b80 in __kernel_vsyscall ()
>   #1  0xf7ec8238 in _exit () from /usr/lib32/libc.so.6
>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> Acked-by: Andy Lutomirski <luto@kernel.org>
> ---
> v8: add WARN_ON_ONCE on current->mm != new_vma->vm_mm
> v7: build fix
> v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function
>     with ifdefs around
> v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are
>     the same, also check not only in_ia32_syscall() but image == &vdso_image_32
> v4: drop __maybe_unused & use image from mm->context instead vdso_image_32
> v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap;
>     used is_ia32_task instead of ifdefs
> v2: added __maybe_unused for pt_regs in vdso_mremap

Ping?

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

* Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
  2016-05-16  9:38 ` [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
@ 2016-05-16 10:54   ` Ingo Molnar
  2016-05-16 11:14     ` Dmitry Safonov
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-05-16 10:54 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46


* Dmitry Safonov <dsafonov@virtuozzo.com> wrote:

> On 05/10/2016 04:29 PM, Dmitry Safonov wrote:
> >Add possibility for userspace 32-bit applications to move
> >vdso mapping. Previously, when userspace app called
> >mremap for vdso, in return path it would land on previous
> >address of vdso page, resulting in segmentation violation.
> >Now it lands fine and returns to userspace with remapped vdso.
> >This will also fix context.vdso pointer for 64-bit, which does not
> >affect the user of vdso after mremap by now, but this may change.
> >
> >As suggested by Andy, return EINVAL for mremap that splits vdso image.
> >
> >Renamed and moved text_mapping structure declaration inside
> >map_vdso, as it used only there and now it complement
> >vvar_mapping variable.
> >
> >There is still problem for remapping vdso in glibc applications:
> >linker relocates addresses for syscalls on vdso page, so
> >you need to relink with the new addresses. Or the next syscall
> >through glibc may fail:
> >  Program received signal SIGSEGV, Segmentation fault.
> >  #0  0xf7fd9b80 in __kernel_vsyscall ()
> >  #1  0xf7ec8238 in _exit () from /usr/lib32/libc.so.6
> >
> >Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> >Acked-by: Andy Lutomirski <luto@kernel.org>
> >---
> >v8: add WARN_ON_ONCE on current->mm != new_vma->vm_mm
> >v7: build fix
> >v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function
> >    with ifdefs around
> >v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are
> >    the same, also check not only in_ia32_syscall() but image == &vdso_image_32
> >v4: drop __maybe_unused & use image from mm->context instead vdso_image_32
> >v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap;
> >    used is_ia32_task instead of ifdefs
> >v2: added __maybe_unused for pt_regs in vdso_mremap
> 
> Ping?

There's no 0/2 boilerplate explaining the background of the changes - why do you 
want to mremap() the vDSO?

Thanks,

	Ingo

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

* Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
  2016-05-16 10:54   ` Ingo Molnar
@ 2016-05-16 11:14     ` Dmitry Safonov
  2016-05-16 13:55       ` Ingo Molnar
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-16 11:14 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46

On 05/16/2016 01:54 PM, Ingo Molnar wrote:
>
> * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>
>> On 05/10/2016 04:29 PM, Dmitry Safonov wrote:
>>> Add possibility for userspace 32-bit applications to move
>>> vdso mapping. Previously, when userspace app called
>>> mremap for vdso, in return path it would land on previous
>>> address of vdso page, resulting in segmentation violation.
>>> Now it lands fine and returns to userspace with remapped vdso.
>>> This will also fix context.vdso pointer for 64-bit, which does not
>>> affect the user of vdso after mremap by now, but this may change.
>>>
>>> As suggested by Andy, return EINVAL for mremap that splits vdso image.
>>>
>>> Renamed and moved text_mapping structure declaration inside
>>> map_vdso, as it used only there and now it complement
>>> vvar_mapping variable.
>>>
>>> There is still problem for remapping vdso in glibc applications:
>>> linker relocates addresses for syscalls on vdso page, so
>>> you need to relink with the new addresses. Or the next syscall
>>> through glibc may fail:
>>>  Program received signal SIGSEGV, Segmentation fault.
>>>  #0  0xf7fd9b80 in __kernel_vsyscall ()
>>>  #1  0xf7ec8238 in _exit () from /usr/lib32/libc.so.6
>>>
>>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>>> Acked-by: Andy Lutomirski <luto@kernel.org>
>>> ---
>>> v8: add WARN_ON_ONCE on current->mm != new_vma->vm_mm
>>> v7: build fix
>>> v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function
>>>    with ifdefs around
>>> v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are
>>>    the same, also check not only in_ia32_syscall() but image == &vdso_image_32
>>> v4: drop __maybe_unused & use image from mm->context instead vdso_image_32
>>> v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap;
>>>    used is_ia32_task instead of ifdefs
>>> v2: added __maybe_unused for pt_regs in vdso_mremap
>>
>> Ping?
>
> There's no 0/2 boilerplate explaining the background of the changes - why do you
> want to mremap() the vDSO?

Thanks for the answer.

Well, one could move vdso vma before this patch, but doing fast
syscalls through it will not work because of code relying on
mm->context.vdso pointer.
So all this code is just fixup for that pointer on moving.
(Also adds preventing for splitting vdso vma).
As Andy notted, vDSO mremap for !i386 tasks also worked only by a chance
before this patch.

I need to move vdso vma in CRIU - on restore we need to choose it's
position:
- if vDSO blob of restoring application is the same as the kernel has,
we need to move it on the same place;
- if it differs, we need to choose place that wasn't tooken by other
vma of restoring application and add jump trampolines to it from the
place of vDSO in restoring application.
And CRIU code now relies on possibility on x86_64 to mremap vDSO.
Without this patch that may be broken in future.
And as I work on C/R of compatible 32-bit applications on x86_64,
I need this to work also for 32-bit vDSO. Which does not work,
because of pointer mentioned above.

Thanks,
Dmitry Safonov

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

* Re: [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test
  2016-05-10 13:29 ` [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test Dmitry Safonov
@ 2016-05-16 13:54   ` Ingo Molnar
  2016-05-16 16:24     ` Dmitry Safonov
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-05-16 13:54 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm,
	0x7f454c46, Shuah Khan, linux-kselftest


* Dmitry Safonov <dsafonov@virtuozzo.com> wrote:

> Should print on success:
> [root@localhost ~]# ./test_mremap_vdso_32
> 	AT_SYSINFO_EHDR is 0xf773f000
> [NOTE]	Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
> [OK]
> Or segfault if landing was bad (before patches):
> [root@localhost ~]# ./test_mremap_vdso_32
> 	AT_SYSINFO_EHDR is 0xf774f000
> [NOTE]	Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
> Segmentation fault (core dumped)

Can the segfault be caught and recovered from, to print a proper failure message?

Thanks,

	Ingo

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

* Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
  2016-05-16 11:14     ` Dmitry Safonov
@ 2016-05-16 13:55       ` Ingo Molnar
  2016-05-16 16:23         ` Dmitry Safonov
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2016-05-16 13:55 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46


* Dmitry Safonov <dsafonov@virtuozzo.com> wrote:

> On 05/16/2016 01:54 PM, Ingo Molnar wrote:
> >
> >* Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
> >
> >>On 05/10/2016 04:29 PM, Dmitry Safonov wrote:
> >>>Add possibility for userspace 32-bit applications to move
> >>>vdso mapping. Previously, when userspace app called
> >>>mremap for vdso, in return path it would land on previous
> >>>address of vdso page, resulting in segmentation violation.
> >>>Now it lands fine and returns to userspace with remapped vdso.
> >>>This will also fix context.vdso pointer for 64-bit, which does not
> >>>affect the user of vdso after mremap by now, but this may change.
> >>>
> >>>As suggested by Andy, return EINVAL for mremap that splits vdso image.
> >>>
> >>>Renamed and moved text_mapping structure declaration inside
> >>>map_vdso, as it used only there and now it complement
> >>>vvar_mapping variable.
> >>>
> >>>There is still problem for remapping vdso in glibc applications:
> >>>linker relocates addresses for syscalls on vdso page, so
> >>>you need to relink with the new addresses. Or the next syscall
> >>>through glibc may fail:
> >>> Program received signal SIGSEGV, Segmentation fault.
> >>> #0  0xf7fd9b80 in __kernel_vsyscall ()
> >>> #1  0xf7ec8238 in _exit () from /usr/lib32/libc.so.6
> >>>
> >>>Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> >>>Acked-by: Andy Lutomirski <luto@kernel.org>
> >>>---
> >>>v8: add WARN_ON_ONCE on current->mm != new_vma->vm_mm
> >>>v7: build fix
> >>>v6: moved vdso_image_32 check and fixup code into vdso_fix_landing function
> >>>   with ifdefs around
> >>>v5: as Andy suggested, add a check that new_vma->vm_mm and current->mm are
> >>>   the same, also check not only in_ia32_syscall() but image == &vdso_image_32
> >>>v4: drop __maybe_unused & use image from mm->context instead vdso_image_32
> >>>v3: as Andy suggested, return EINVAL in case of splitting vdso blob on mremap;
> >>>   used is_ia32_task instead of ifdefs
> >>>v2: added __maybe_unused for pt_regs in vdso_mremap
> >>
> >>Ping?
> >
> >There's no 0/2 boilerplate explaining the background of the changes - why do you
> >want to mremap() the vDSO?
> 
> Thanks for the answer.
> 
> Well, one could move vdso vma before this patch, but doing fast
> syscalls through it will not work because of code relying on
> mm->context.vdso pointer.
> So all this code is just fixup for that pointer on moving.
> (Also adds preventing for splitting vdso vma).
> As Andy notted, vDSO mremap for !i386 tasks also worked only by a chance
> before this patch.
> 
> I need to move vdso vma in CRIU - on restore we need to choose it's
> position:
> - if vDSO blob of restoring application is the same as the kernel has,
> we need to move it on the same place;
> - if it differs, we need to choose place that wasn't tooken by other
> vma of restoring application and add jump trampolines to it from the
> place of vDSO in restoring application.
> And CRIU code now relies on possibility on x86_64 to mremap vDSO.
> Without this patch that may be broken in future.
> And as I work on C/R of compatible 32-bit applications on x86_64,
> I need this to work also for 32-bit vDSO. Which does not work,
> because of pointer mentioned above.

Ok, this looks useful - please add this information to the changelog (with typos 
fixed).

Thanks,

	Ingo

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

* Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping
  2016-05-16 13:55       ` Ingo Molnar
@ 2016-05-16 16:23         ` Dmitry Safonov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-16 16:23 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm, 0x7f454c46

On 05/16/2016 04:55 PM, Ingo Molnar wrote:
>
>
> Ok, this looks useful - please add this information to the changelog (with typos
> fixed).

Thanks will add to v9.

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

* Re: [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test
  2016-05-16 13:54   ` Ingo Molnar
@ 2016-05-16 16:24     ` Dmitry Safonov
  2016-05-16 18:25       ` Andy Lutomirski
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-16 16:24 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, mingo, luto, tglx, hpa, x86, akpm, linux-mm,
	0x7f454c46, Shuah Khan, linux-kselftest

On 05/16/2016 04:54 PM, Ingo Molnar wrote:
>
> * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>
>> Should print on success:
>> [root@localhost ~]# ./test_mremap_vdso_32
>> 	AT_SYSINFO_EHDR is 0xf773f000
>> [NOTE]	Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
>> [OK]
>> Or segfault if landing was bad (before patches):
>> [root@localhost ~]# ./test_mremap_vdso_32
>> 	AT_SYSINFO_EHDR is 0xf774f000
>> [NOTE]	Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
>> Segmentation fault (core dumped)
>
> Can the segfault be caught and recovered from, to print a proper failure message?

Will add segfault handler, thanks.

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

* Re: [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test
  2016-05-16 16:24     ` Dmitry Safonov
@ 2016-05-16 18:25       ` Andy Lutomirski
  2016-05-17 10:25         ` Dmitry Safonov
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Lutomirski @ 2016-05-16 18:25 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, X86 ML, Andrew Morton, linux-mm, Dmitry Safonov,
	Shuah Khan, linux-kselftest

On Mon, May 16, 2016 at 9:24 AM, Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
> On 05/16/2016 04:54 PM, Ingo Molnar wrote:
>>
>>
>> * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>>
>>> Should print on success:
>>> [root@localhost ~]# ./test_mremap_vdso_32
>>>         AT_SYSINFO_EHDR is 0xf773f000
>>> [NOTE]  Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
>>> [OK]
>>> Or segfault if landing was bad (before patches):
>>> [root@localhost ~]# ./test_mremap_vdso_32
>>>         AT_SYSINFO_EHDR is 0xf774f000
>>> [NOTE]  Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
>>> Segmentation fault (core dumped)
>>
>>
>> Can the segfault be caught and recovered from, to print a proper failure
>> message?
>
>
> Will add segfault handler, thanks.
>

It may be more complicated that that.  Glibc is likely to explode if
this happens, and the headers are sufficiently screwed up that it's
awkward to bypass glibc and call rt_sigaction directly.  I have a test
that does the latter, though, so it's at least possible, but I'm
unconvinced it's worth it just for an error message.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

* Re: [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test
  2016-05-16 18:25       ` Andy Lutomirski
@ 2016-05-17 10:25         ` Dmitry Safonov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Safonov @ 2016-05-17 10:25 UTC (permalink / raw)
  To: Andy Lutomirski, Ingo Molnar
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	X86 ML, Andrew Morton, linux-mm, Dmitry Safonov, Shuah Khan,
	linux-kselftest

On 05/16/2016 09:25 PM, Andy Lutomirski wrote:
> On Mon, May 16, 2016 at 9:24 AM, Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>> On 05/16/2016 04:54 PM, Ingo Molnar wrote:
>>>
>>>
>>> * Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
>>>
>>>> Should print on success:
>>>> [root@localhost ~]# ./test_mremap_vdso_32
>>>>         AT_SYSINFO_EHDR is 0xf773f000
>>>> [NOTE]  Moving vDSO: [f773f000, f7740000] -> [a000000, a001000]
>>>> [OK]
>>>> Or segfault if landing was bad (before patches):
>>>> [root@localhost ~]# ./test_mremap_vdso_32
>>>>         AT_SYSINFO_EHDR is 0xf774f000
>>>> [NOTE]  Moving vDSO: [f774f000, f7750000] -> [a000000, a001000]
>>>> Segmentation fault (core dumped)
>>>
>>>
>>> Can the segfault be caught and recovered from, to print a proper failure
>>> message?
>>
>>
>> Will add segfault handler, thanks.
>>
>
> It may be more complicated that that.  Glibc is likely to explode if
> this happens, and the headers are sufficiently screwed up that it's
> awkward to bypass glibc and call rt_sigaction directly.  I have a test
> that does the latter, though, so it's at least possible, but I'm
> unconvinced it's worth it just for an error message.

Oh, I didn't know that, thanks, Andy.
I'll leave it as is for simplicity.

-- 
Regards,
Dmitry Safonov

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

end of thread, other threads:[~2016-05-17 10:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-10 13:29 [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
2016-05-10 13:29 ` [PATCHv8 resend 2/2] selftest/x86: add mremap vdso test Dmitry Safonov
2016-05-16 13:54   ` Ingo Molnar
2016-05-16 16:24     ` Dmitry Safonov
2016-05-16 18:25       ` Andy Lutomirski
2016-05-17 10:25         ` Dmitry Safonov
2016-05-16  9:38 ` [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping Dmitry Safonov
2016-05-16 10:54   ` Ingo Molnar
2016-05-16 11:14     ` Dmitry Safonov
2016-05-16 13:55       ` Ingo Molnar
2016-05-16 16:23         ` Dmitry Safonov

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