Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1401403 > unrolled thread

Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to  vm_special_mapping Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-05-16 12:00 +0200
    Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to  vm_special_mapping Ingo Molnar <mingo@kernel.org> - 2016-05-16 13:00 +0200
      Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to  vm_special_mapping Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-05-16 13:20 +0200
        Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to  vm_special_mapping Ingo Molnar <mingo@kernel.org> - 2016-05-16 16:00 +0200
          Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to  vm_special_mapping Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-05-16 18:30 +0200

#1401403 — Re: [PATCHv8 resend 1/2] x86/vdso: add mremap hook to vm_special_mapping

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?

[toc] | [next] | [standalone]

#1401434

* 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

[toc] | [prev] | [next] | [standalone]

#1401440

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

[toc] | [prev] | [next] | [standalone]

#1401511

* 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

[toc] | [prev] | [next] | [standalone]

#1401613

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web