| Message ID | 1404930133-30324-2-git-send-email-will.deacon@arm.com |
|---|---|
| State | Accepted |
| Commit | 8715493852783358ef8656a0054a14bf822509cf |
| Headers | show |
Hi Will, one question (and maybe it's more of a question for Andy than for you). On 07/09/2014 01:22 PM, Will Deacon wrote: > - ret = install_special_mapping(mm, vdso_base, vdso_mapping_len, > + ret = install_special_mapping(mm, vdso_base, vdso_text_len, > VM_READ|VM_EXEC| > VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, > vdso_pagelist); > - if (ret) { > - mm->context.vdso = NULL; > + if (ret) > + goto up_fail; > + > + vdso_base += vdso_text_len; > + ret = install_special_mapping(mm, vdso_base, PAGE_SIZE, > + VM_READ|VM_MAYREAD, > + vdso_pagelist + vdso_pages); I note this code sets VM_MAYREAD for the data page, while x86's vvar mapping sets only VM_READ. (Personally I am not totally clear on the meaning of VM_MAYREAD, but it looks like it would have implications for use of mprotect on the mapping.) Should one arch or the other adjust its flags?
On Wed, Jul 9, 2014 at 12:44 PM, Nathan Lynch <Nathan_Lynch@mentor.com> wrote: > Hi Will, one question (and maybe it's more of a question for Andy than > for you). > > On 07/09/2014 01:22 PM, Will Deacon wrote: >> - ret = install_special_mapping(mm, vdso_base, vdso_mapping_len, >> + ret = install_special_mapping(mm, vdso_base, vdso_text_len, >> VM_READ|VM_EXEC| >> VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, >> vdso_pagelist); >> - if (ret) { >> - mm->context.vdso = NULL; >> + if (ret) >> + goto up_fail; >> + >> + vdso_base += vdso_text_len; >> + ret = install_special_mapping(mm, vdso_base, PAGE_SIZE, >> + VM_READ|VM_MAYREAD, >> + vdso_pagelist + vdso_pages); > > I note this code sets VM_MAYREAD for the data page, while x86's vvar > mapping sets only VM_READ. (Personally I am not totally clear on the > meaning of VM_MAYREAD, but it looks like it would have implications for > use of mprotect on the mapping.) Should one arch or the other adjust > its flags? > On quick inspection, this doesn't matter. That being said, VM_MAYREAD seems reasonable here. I'll queue up the x86 change. --Andy
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index 50384fec56c4..84cafbc3eb54 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -138,11 +138,12 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) { struct mm_struct *mm = current->mm; - unsigned long vdso_base, vdso_mapping_len; + unsigned long vdso_base, vdso_text_len, vdso_mapping_len; int ret; + vdso_text_len = vdso_pages << PAGE_SHIFT; /* Be sure to map the data page */ - vdso_mapping_len = (vdso_pages + 1) << PAGE_SHIFT; + vdso_mapping_len = vdso_text_len + PAGE_SIZE; down_write(&mm->mmap_sem); vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0); @@ -152,35 +153,52 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, } mm->context.vdso = (void *)vdso_base; - ret = install_special_mapping(mm, vdso_base, vdso_mapping_len, + ret = install_special_mapping(mm, vdso_base, vdso_text_len, VM_READ|VM_EXEC| VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC, vdso_pagelist); - if (ret) { - mm->context.vdso = NULL; + if (ret) + goto up_fail; + + vdso_base += vdso_text_len; + ret = install_special_mapping(mm, vdso_base, PAGE_SIZE, + VM_READ|VM_MAYREAD, + vdso_pagelist + vdso_pages); + if (ret) goto up_fail; - } -up_fail: up_write(&mm->mmap_sem); + return 0; +up_fail: + mm->context.vdso = NULL; + up_write(&mm->mmap_sem); return ret; } const char *arch_vma_name(struct vm_area_struct *vma) { + unsigned long vdso_text; + + if (!vma->vm_mm) + return NULL; + + vdso_text = (unsigned long)vma->vm_mm->context.vdso; + /* * We can re-use the vdso pointer in mm_context_t for identifying * the vectors page for compat applications. The vDSO will always * sit above TASK_UNMAPPED_BASE and so we don't need to worry about * it conflicting with the vectors base. */ - if (vma->vm_mm && vma->vm_start == (long)vma->vm_mm->context.vdso) { + if (vma->vm_start == vdso_text) { #ifdef CONFIG_COMPAT if (vma->vm_start == AARCH32_VECTORS_BASE) return "[vectors]"; #endif return "[vdso]"; + } else if (vma->vm_start == (vdso_text + (vdso_pages << PAGE_SHIFT))) { + return "[vvar]"; } return NULL;
The VDSO datapage doesn't need to be executable (no code there) or CoW-able (the kernel writes the page, so a private copy is totally useless). This patch moves the datapage into its own VMA, identified as "[vvar]" in /proc/<pid>/maps. Cc: Andy Lutomirski <luto@amacapital.net> Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm64/kernel/vdso.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)