linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64/vdso: Support mremap() for vDSO
@ 2017-07-26 17:07 Dmitry Safonov
  2017-07-28 16:48 ` Will Deacon
  2017-08-09 10:13 ` Catalin Marinas
  0 siblings, 2 replies; 8+ messages in thread
From: Dmitry Safonov @ 2017-07-26 17:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: 0x7f454c46, Dmitry Safonov, Catalin Marinas, Will Deacon,
	Russell King, linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

vDSO VMA address is saved in mm_context for the purpose of using
restorer from vDSO page to return to userspace after signal handling.

In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
on restore back to the place where it was on the dump.
With the exception for x86 (where there is API to map vDSO with
arch_prctl()), we move vDSO inherited from CRIU task to restoree
position by mremap().

CRIU does support arm64 architecture, but kernel doesn't update
context.vdso pointer after mremap(). Which results in translation
fault after signal handling on restored application:
https://github.com/xemul/criu/issues/288

Make vDSO code track the VMA address by supplying .mremap() fops
the same way it's done for x86 and arm32 by:
commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@virtuozzo.com>
Cc: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
 arch/arm64/kernel/vdso.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index e8f759f764f2..2d419006ad43 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
 }
 #endif /* CONFIG_COMPAT */
 
+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;
+	unsigned long vdso_size = vdso_end - vdso_start;
+
+	if (vdso_size != new_size)
+		return -EINVAL;
+
+	current->mm->context.vdso = (void *)new_vma->vm_start;
+
+	return 0;
+}
+
 static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
 	{
 		.name	= "[vvar]",
 	},
 	{
 		.name	= "[vdso]",
+		.mremap = vdso_mremap,
 	},
 };
 
-- 
2.13.3

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-07-26 17:07 [PATCH] arm64/vdso: Support mremap() for vDSO Dmitry Safonov
@ 2017-07-28 16:48 ` Will Deacon
  2017-07-28 19:06   ` Dmitry Safonov
  2017-08-09 10:13 ` Catalin Marinas
  1 sibling, 1 reply; 8+ messages in thread
From: Will Deacon @ 2017-07-28 16:48 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, 0x7f454c46, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
> vDSO VMA address is saved in mm_context for the purpose of using
> restorer from vDSO page to return to userspace after signal handling.
> 
> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
> on restore back to the place where it was on the dump.
> With the exception for x86 (where there is API to map vDSO with
> arch_prctl()), we move vDSO inherited from CRIU task to restoree
> position by mremap().
> 
> CRIU does support arm64 architecture, but kernel doesn't update
> context.vdso pointer after mremap(). Which results in translation
> fault after signal handling on restored application:
> https://github.com/xemul/criu/issues/288
> 
> Make vDSO code track the VMA address by supplying .mremap() fops
> the same way it's done for x86 and arm32 by:
> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Cc: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---
>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index e8f759f764f2..2d419006ad43 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
>  }
>  #endif /* CONFIG_COMPAT */
>  
> +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;
> +	unsigned long vdso_size = vdso_end - vdso_start;

You might be able to use vdso_pages here, but it depends on my question
below.

> +
> +	if (vdso_size != new_size)
> +		return -EINVAL;
> +
> +	current->mm->context.vdso = (void *)new_vma->vm_start;
> +
> +	return 0;
> +}
> +
>  static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
>  	{
>  		.name	= "[vvar]",
>  	},
>  	{
>  		.name	= "[vdso]",
> +		.mremap = vdso_mremap,

Does this mean we move the vdso text, but not the data page? How does that
work?

Will

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-07-28 16:48 ` Will Deacon
@ 2017-07-28 19:06   ` Dmitry Safonov
  2017-08-02 16:04     ` Will Deacon
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Safonov @ 2017-07-28 19:06 UTC (permalink / raw)
  To: Will Deacon
  Cc: Dmitry Safonov, open list, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

2017-07-28 19:48 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
>> vDSO VMA address is saved in mm_context for the purpose of using
>> restorer from vDSO page to return to userspace after signal handling.
>>
>> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
>> on restore back to the place where it was on the dump.
>> With the exception for x86 (where there is API to map vDSO with
>> arch_prctl()), we move vDSO inherited from CRIU task to restoree
>> position by mremap().
>>
>> CRIU does support arm64 architecture, but kernel doesn't update
>> context.vdso pointer after mremap(). Which results in translation
>> fault after signal handling on restored application:
>> https://github.com/xemul/criu/issues/288
>>
>> Make vDSO code track the VMA address by supplying .mremap() fops
>> the same way it's done for x86 and arm32 by:
>> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Russell King <rmk+kernel@armlinux.org.uk>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>> Cc: Christopher Covington <cov@codeaurora.org>
>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>> ---
>>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index e8f759f764f2..2d419006ad43 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
>>  }
>>  #endif /* CONFIG_COMPAT */
>>
>> +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;
>> +     unsigned long vdso_size = vdso_end - vdso_start;
>
> You might be able to use vdso_pages here, but it depends on my question
> below.

Yes, shifting with PAGE_SHIFT.
Is it just a preference?

>
>> +
>> +     if (vdso_size != new_size)
>> +             return -EINVAL;
>> +
>> +     current->mm->context.vdso = (void *)new_vma->vm_start;
>> +
>> +     return 0;
>> +}
>> +
>>  static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
>>       {
>>               .name   = "[vvar]",
>>       },
>>       {
>>               .name   = "[vdso]",
>> +             .mremap = vdso_mremap,
>
> Does this mean we move the vdso text, but not the data page? How does that
> work?

Well, the kernel tracks only vdso pages - to find restorer addr after a signal.
In userspace one needs to move vvar and vdso vma pair accordingly,
with the same order and offset of course.

-- 
             Dmitry

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-07-28 19:06   ` Dmitry Safonov
@ 2017-08-02 16:04     ` Will Deacon
  2017-08-08  9:29       ` Dmitry Safonov
  0 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2017-08-02 16:04 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Dmitry Safonov, open list, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

On Fri, Jul 28, 2017 at 10:06:20PM +0300, Dmitry Safonov wrote:
> 2017-07-28 19:48 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> > On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
> >> vDSO VMA address is saved in mm_context for the purpose of using
> >> restorer from vDSO page to return to userspace after signal handling.
> >>
> >> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
> >> on restore back to the place where it was on the dump.
> >> With the exception for x86 (where there is API to map vDSO with
> >> arch_prctl()), we move vDSO inherited from CRIU task to restoree
> >> position by mremap().
> >>
> >> CRIU does support arm64 architecture, but kernel doesn't update
> >> context.vdso pointer after mremap(). Which results in translation
> >> fault after signal handling on restored application:
> >> https://github.com/xemul/criu/issues/288
> >>
> >> Make vDSO code track the VMA address by supplying .mremap() fops
> >> the same way it's done for x86 and arm32 by:
> >> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
> >> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will.deacon@arm.com>
> >> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> >> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> >> Cc: Christopher Covington <cov@codeaurora.org>
> >> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> >> ---
> >>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> >> index e8f759f764f2..2d419006ad43 100644
> >> --- a/arch/arm64/kernel/vdso.c
> >> +++ b/arch/arm64/kernel/vdso.c
> >> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
> >>  }
> >>  #endif /* CONFIG_COMPAT */
> >>
> >> +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;
> >> +     unsigned long vdso_size = vdso_end - vdso_start;
> >
> > You might be able to use vdso_pages here, but it depends on my question
> > below.
> 
> Yes, shifting with PAGE_SHIFT.
> Is it just a preference?

Yeah, just a minor thing, although thinking about it again, I don't know
what you're trying to achieve with the size check anyway. Userspace is only
going to hurt itself if it screws up the layout, so why police this?

> >
> >> +
> >> +     if (vdso_size != new_size)
> >> +             return -EINVAL;
> >> +
> >> +     current->mm->context.vdso = (void *)new_vma->vm_start;
> >> +
> >> +     return 0;
> >> +}
> >> +
> >>  static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
> >>       {
> >>               .name   = "[vvar]",
> >>       },
> >>       {
> >>               .name   = "[vdso]",
> >> +             .mremap = vdso_mremap,
> >
> > Does this mean we move the vdso text, but not the data page? How does that
> > work?
> 
> Well, the kernel tracks only vdso pages - to find restorer addr after a signal.
> In userspace one needs to move vvar and vdso vma pair accordingly,
> with the same order and offset of course.

Ah, I see. I misunderstood what the .mremap callback was actually doing.
I guess there's also no issue with not being able to do this atomically,
either, as long as you can avoid making syscalls via the vDSO until you've
relocated both mappings.

Will

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-08-02 16:04     ` Will Deacon
@ 2017-08-08  9:29       ` Dmitry Safonov
  2017-08-08  9:44         ` Will Deacon
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Safonov @ 2017-08-08  9:29 UTC (permalink / raw)
  To: Will Deacon
  Cc: Dmitry Safonov, open list, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

2017-08-02 19:04 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> On Fri, Jul 28, 2017 at 10:06:20PM +0300, Dmitry Safonov wrote:
>> 2017-07-28 19:48 GMT+03:00 Will Deacon <will.deacon@arm.com>:
>> > On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
>> >> vDSO VMA address is saved in mm_context for the purpose of using
>> >> restorer from vDSO page to return to userspace after signal handling.
>> >>
>> >> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
>> >> on restore back to the place where it was on the dump.
>> >> With the exception for x86 (where there is API to map vDSO with
>> >> arch_prctl()), we move vDSO inherited from CRIU task to restoree
>> >> position by mremap().
>> >>
>> >> CRIU does support arm64 architecture, but kernel doesn't update
>> >> context.vdso pointer after mremap(). Which results in translation
>> >> fault after signal handling on restored application:
>> >> https://github.com/xemul/criu/issues/288
>> >>
>> >> Make vDSO code track the VMA address by supplying .mremap() fops
>> >> the same way it's done for x86 and arm32 by:
>> >> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>> >> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
>> >>
>> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> >> Cc: Will Deacon <will.deacon@arm.com>
>> >> Cc: Russell King <rmk+kernel@armlinux.org.uk>
>> >> Cc: linux-arm-kernel@lists.infradead.org
>> >> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>> >> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>> >> Cc: Christopher Covington <cov@codeaurora.org>
>> >> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>> >> ---
>> >>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
>> >>  1 file changed, 15 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> >> index e8f759f764f2..2d419006ad43 100644
>> >> --- a/arch/arm64/kernel/vdso.c
>> >> +++ b/arch/arm64/kernel/vdso.c
>> >> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
>> >>  }
>> >>  #endif /* CONFIG_COMPAT */
>> >>
>> >> +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;
>> >> +     unsigned long vdso_size = vdso_end - vdso_start;
>> >
>> > You might be able to use vdso_pages here, but it depends on my question
>> > below.
>>
>> Yes, shifting with PAGE_SHIFT.
>> Is it just a preference?
>
> Yeah, just a minor thing, although thinking about it again, I don't know
> what you're trying to achieve with the size check anyway. Userspace is only
> going to hurt itself if it screws up the layout, so why police this?

Well, it's for keeping the same semantics as on x86.
The idea of restriction to partial mremap() is suggested by Andy
so that userspace won't be allowed to hurt itself and to simplify
kernel code on x86.

>
>> >
>> >> +
>> >> +     if (vdso_size != new_size)
>> >> +             return -EINVAL;
>> >> +
>> >> +     current->mm->context.vdso = (void *)new_vma->vm_start;
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >>  static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
>> >>       {
>> >>               .name   = "[vvar]",
>> >>       },
>> >>       {
>> >>               .name   = "[vdso]",
>> >> +             .mremap = vdso_mremap,
>> >
>> > Does this mean we move the vdso text, but not the data page? How does that
>> > work?
>>
>> Well, the kernel tracks only vdso pages - to find restorer addr after a signal.
>> In userspace one needs to move vvar and vdso vma pair accordingly,
>> with the same order and offset of course.
>
> Ah, I see. I misunderstood what the .mremap callback was actually doing.
> I guess there's also no issue with not being able to do this atomically,
> either, as long as you can avoid making syscalls via the vDSO until you've
> relocated both mappings.

Yes, also one should keep in mind that there might be some linker
work needed after mremap() of vdso. As some loaded libraries may
have been linked to vdso's vma addresses.

In CRIU we have `restorer' code which is PIE code, non-linked to
libc and calling raw syscalls rather than using vdso. And restoree
(the application being restored) is already linked to vdso's new position.

-- 
             Dmitry

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-08-08  9:29       ` Dmitry Safonov
@ 2017-08-08  9:44         ` Will Deacon
  2017-08-08 17:02           ` Dmitry Safonov
  0 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2017-08-08  9:44 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: Dmitry Safonov, open list, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

On Tue, Aug 08, 2017 at 12:29:50PM +0300, Dmitry Safonov wrote:
> 2017-08-02 19:04 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> > On Fri, Jul 28, 2017 at 10:06:20PM +0300, Dmitry Safonov wrote:
> >> 2017-07-28 19:48 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> >> > On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
> >> >> vDSO VMA address is saved in mm_context for the purpose of using
> >> >> restorer from vDSO page to return to userspace after signal handling.
> >> >>
> >> >> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
> >> >> on restore back to the place where it was on the dump.
> >> >> With the exception for x86 (where there is API to map vDSO with
> >> >> arch_prctl()), we move vDSO inherited from CRIU task to restoree
> >> >> position by mremap().
> >> >>
> >> >> CRIU does support arm64 architecture, but kernel doesn't update
> >> >> context.vdso pointer after mremap(). Which results in translation
> >> >> fault after signal handling on restored application:
> >> >> https://github.com/xemul/criu/issues/288
> >> >>
> >> >> Make vDSO code track the VMA address by supplying .mremap() fops
> >> >> the same way it's done for x86 and arm32 by:
> >> >> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
> >> >> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
> >> >>
> >> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> >> Cc: Will Deacon <will.deacon@arm.com>
> >> >> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> >> >> Cc: linux-arm-kernel@lists.infradead.org
> >> >> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> >> >> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> >> >> Cc: Christopher Covington <cov@codeaurora.org>
> >> >> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> >> >> ---
> >> >>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
> >> >>  1 file changed, 15 insertions(+)
> >> >>
> >> >> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> >> >> index e8f759f764f2..2d419006ad43 100644
> >> >> --- a/arch/arm64/kernel/vdso.c
> >> >> +++ b/arch/arm64/kernel/vdso.c
> >> >> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
> >> >>  }
> >> >>  #endif /* CONFIG_COMPAT */
> >> >>
> >> >> +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;
> >> >> +     unsigned long vdso_size = vdso_end - vdso_start;
> >> >
> >> > You might be able to use vdso_pages here, but it depends on my question
> >> > below.
> >>
> >> Yes, shifting with PAGE_SHIFT.
> >> Is it just a preference?
> >
> > Yeah, just a minor thing, although thinking about it again, I don't know
> > what you're trying to achieve with the size check anyway. Userspace is only
> > going to hurt itself if it screws up the layout, so why police this?
> 
> Well, it's for keeping the same semantics as on x86.
> The idea of restriction to partial mremap() is suggested by Andy
> so that userspace won't be allowed to hurt itself and to simplify
> kernel code on x86.

I still don't see why that's a useful thing for us to be doing on arm64, but
ok.

Either way:

Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-08-08  9:44         ` Will Deacon
@ 2017-08-08 17:02           ` Dmitry Safonov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Safonov @ 2017-08-08 17:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: Dmitry Safonov, open list, Catalin Marinas, Russell King,
	linux-arm-kernel, Cyrill Gorcunov, Pavel Emelyanov,
	Christopher Covington

2017-08-08 12:44 GMT+03:00 Will Deacon <will.deacon@arm.com>:
> On Tue, Aug 08, 2017 at 12:29:50PM +0300, Dmitry Safonov wrote:
>> 2017-08-02 19:04 GMT+03:00 Will Deacon <will.deacon@arm.com>:
>> > On Fri, Jul 28, 2017 at 10:06:20PM +0300, Dmitry Safonov wrote:
>> >> 2017-07-28 19:48 GMT+03:00 Will Deacon <will.deacon@arm.com>:
>> >> > On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
>> >> >> vDSO VMA address is saved in mm_context for the purpose of using
>> >> >> restorer from vDSO page to return to userspace after signal handling.
>> >> >>
>> >> >> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
>> >> >> on restore back to the place where it was on the dump.
>> >> >> With the exception for x86 (where there is API to map vDSO with
>> >> >> arch_prctl()), we move vDSO inherited from CRIU task to restoree
>> >> >> position by mremap().
>> >> >>
>> >> >> CRIU does support arm64 architecture, but kernel doesn't update
>> >> >> context.vdso pointer after mremap(). Which results in translation
>> >> >> fault after signal handling on restored application:
>> >> >> https://github.com/xemul/criu/issues/288
>> >> >>
>> >> >> Make vDSO code track the VMA address by supplying .mremap() fops
>> >> >> the same way it's done for x86 and arm32 by:
>> >> >> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
>> >> >> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
>> >> >>
>> >> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> >> >> Cc: Will Deacon <will.deacon@arm.com>
>> >> >> Cc: Russell King <rmk+kernel@armlinux.org.uk>
>> >> >> Cc: linux-arm-kernel@lists.infradead.org
>> >> >> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>> >> >> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>> >> >> Cc: Christopher Covington <cov@codeaurora.org>
>> >> >> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>> >> >> ---
>> >> >>  arch/arm64/kernel/vdso.c | 15 +++++++++++++++
>> >> >>  1 file changed, 15 insertions(+)
>> >> >>
>> >> >> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> >> >> index e8f759f764f2..2d419006ad43 100644
>> >> >> --- a/arch/arm64/kernel/vdso.c
>> >> >> +++ b/arch/arm64/kernel/vdso.c
>> >> >> @@ -110,12 +110,27 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
>> >> >>  }
>> >> >>  #endif /* CONFIG_COMPAT */
>> >> >>
>> >> >> +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;
>> >> >> +     unsigned long vdso_size = vdso_end - vdso_start;
>> >> >
>> >> > You might be able to use vdso_pages here, but it depends on my question
>> >> > below.
>> >>
>> >> Yes, shifting with PAGE_SHIFT.
>> >> Is it just a preference?
>> >
>> > Yeah, just a minor thing, although thinking about it again, I don't know
>> > what you're trying to achieve with the size check anyway. Userspace is only
>> > going to hurt itself if it screws up the layout, so why police this?
>>
>> Well, it's for keeping the same semantics as on x86.
>> The idea of restriction to partial mremap() is suggested by Andy
>> so that userspace won't be allowed to hurt itself and to simplify
>> kernel code on x86.
>
> I still don't see why that's a useful thing for us to be doing on arm64, but
> ok.
>
> Either way:
>
> Reviewed-by: Will Deacon <will.deacon@arm.com>

Thanks!

-- 
             Dmitry

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

* Re: [PATCH] arm64/vdso: Support mremap() for vDSO
  2017-07-26 17:07 [PATCH] arm64/vdso: Support mremap() for vDSO Dmitry Safonov
  2017-07-28 16:48 ` Will Deacon
@ 2017-08-09 10:13 ` Catalin Marinas
  1 sibling, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2017-08-09 10:13 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, 0x7f454c46, Will Deacon, Russell King,
	Christopher Covington, Cyrill Gorcunov, linux-arm-kernel,
	Pavel Emelyanov

On Wed, Jul 26, 2017 at 08:07:37PM +0300, Dmitry Safonov wrote:
> vDSO VMA address is saved in mm_context for the purpose of using
> restorer from vDSO page to return to userspace after signal handling.
> 
> In Checkpoint Restore in Userspace (CRIU) project we place vDSO VMA
> on restore back to the place where it was on the dump.
> With the exception for x86 (where there is API to map vDSO with
> arch_prctl()), we move vDSO inherited from CRIU task to restoree
> position by mremap().
> 
> CRIU does support arm64 architecture, but kernel doesn't update
> context.vdso pointer after mremap(). Which results in translation
> fault after signal handling on restored application:
> https://github.com/xemul/criu/issues/288
> 
> Make vDSO code track the VMA address by supplying .mremap() fops
> the same way it's done for x86 and arm32 by:
> commit b059a453b1cf ("x86/vdso: Add mremap hook to vm_special_mapping")
> commit 280e87e98c09 ("ARM: 8683/1: ARM32: Support mremap() for sigpage/vDSO").
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Cc: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>

Queued for 4.14. Thanks.

-- 
Catalin

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

end of thread, other threads:[~2017-08-09 10:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 17:07 [PATCH] arm64/vdso: Support mremap() for vDSO Dmitry Safonov
2017-07-28 16:48 ` Will Deacon
2017-07-28 19:06   ` Dmitry Safonov
2017-08-02 16:04     ` Will Deacon
2017-08-08  9:29       ` Dmitry Safonov
2017-08-08  9:44         ` Will Deacon
2017-08-08 17:02           ` Dmitry Safonov
2017-08-09 10:13 ` Catalin Marinas

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