* [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma
2014-07-09 18:22 [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Will Deacon
@ 2014-07-09 18:22 ` Will Deacon
2014-07-09 19:44 ` Nathan Lynch
2014-07-09 18:22 ` [PATCH 2/3] arm64: vdso: move to _install_special_mapping and remove arch_vma_name Will Deacon
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2014-07-09 18:22 UTC (permalink / raw)
To: linux-arm-kernel
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(-)
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;
--
2.0.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma
2014-07-09 18:22 ` [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma Will Deacon
@ 2014-07-09 19:44 ` Nathan Lynch
2014-07-09 19:48 ` Andy Lutomirski
0 siblings, 1 reply; 7+ messages in thread
From: Nathan Lynch @ 2014-07-09 19:44 UTC (permalink / raw)
To: linux-arm-kernel
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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma
2014-07-09 19:44 ` Nathan Lynch
@ 2014-07-09 19:48 ` Andy Lutomirski
0 siblings, 0 replies; 7+ messages in thread
From: Andy Lutomirski @ 2014-07-09 19:48 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] arm64: vdso: move to _install_special_mapping and remove arch_vma_name
2014-07-09 18:22 [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Will Deacon
2014-07-09 18:22 ` [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma Will Deacon
@ 2014-07-09 18:22 ` Will Deacon
2014-07-09 18:22 ` [PATCH 3/3] arm64: vdso: move data page before code pages Will Deacon
2014-07-24 18:20 ` [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Christopher Covington
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2014-07-09 18:22 UTC (permalink / raw)
To: linux-arm-kernel
_install_special_mapping replaces install_special_mapping and removes
the need to detect special VMA in arch_vma_name.
This patch moves the vdso and compat vectors page code over to the new
API.
Cc: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/vdso.c | 80 +++++++++++++++++++++---------------------------
1 file changed, 35 insertions(+), 45 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 84cafbc3eb54..60ae12087d9f 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -88,22 +88,29 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
{
struct mm_struct *mm = current->mm;
unsigned long addr = AARCH32_VECTORS_BASE;
- int ret;
+ static struct vm_special_mapping spec = {
+ .name = "[vectors]",
+ .pages = vectors_page,
+
+ };
+ void *ret;
down_write(&mm->mmap_sem);
current->mm->context.vdso = (void *)addr;
/* Map vectors page at the high address. */
- ret = install_special_mapping(mm, addr, PAGE_SIZE,
- VM_READ|VM_EXEC|VM_MAYREAD|VM_MAYEXEC,
- vectors_page);
+ ret = _install_special_mapping(mm, addr, PAGE_SIZE,
+ VM_READ|VM_EXEC|VM_MAYREAD|VM_MAYEXEC,
+ &spec);
up_write(&mm->mmap_sem);
- return ret;
+ return PTR_ERR_OR_ZERO(ret);
}
#endif /* CONFIG_COMPAT */
+static struct vm_special_mapping vdso_spec[2];
+
static int __init vdso_init(void)
{
int i;
@@ -130,6 +137,17 @@ static int __init vdso_init(void)
/* Grab the vDSO data page. */
vdso_pagelist[i] = virt_to_page(vdso_data);
+ /* Populate the special mapping structures */
+ vdso_spec[0] = (struct vm_special_mapping) {
+ .name = "[vdso]",
+ .pages = vdso_pagelist,
+ };
+
+ vdso_spec[1] = (struct vm_special_mapping) {
+ .name = "[vvar]",
+ .pages = vdso_pagelist + vdso_pages,
+ };
+
return 0;
}
arch_initcall(vdso_init);
@@ -139,7 +157,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
{
struct mm_struct *mm = current->mm;
unsigned long vdso_base, vdso_text_len, vdso_mapping_len;
- int ret;
+ void *ret;
vdso_text_len = vdso_pages << PAGE_SHIFT;
/* Be sure to map the data page */
@@ -148,23 +166,23 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
down_write(&mm->mmap_sem);
vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
if (IS_ERR_VALUE(vdso_base)) {
- ret = vdso_base;
+ ret = ERR_PTR(vdso_base);
goto up_fail;
}
mm->context.vdso = (void *)vdso_base;
- ret = install_special_mapping(mm, vdso_base, vdso_text_len,
- VM_READ|VM_EXEC|
- VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
- vdso_pagelist);
- if (ret)
+ ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
+ VM_READ|VM_EXEC|
+ VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
+ &vdso_spec[0]);
+ if (IS_ERR(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)
+ ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
+ VM_READ|VM_MAYREAD,
+ &vdso_spec[1]);
+ if (IS_ERR(ret))
goto up_fail;
up_write(&mm->mmap_sem);
@@ -173,35 +191,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
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_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;
+ return PTR_ERR(ret);
}
/*
--
2.0.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] arm64: vdso: move data page before code pages
2014-07-09 18:22 [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Will Deacon
2014-07-09 18:22 ` [PATCH 1/3] arm64: vdso: put vdso datapage in a separate vma Will Deacon
2014-07-09 18:22 ` [PATCH 2/3] arm64: vdso: move to _install_special_mapping and remove arch_vma_name Will Deacon
@ 2014-07-09 18:22 ` Will Deacon
2014-07-24 18:20 ` [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Christopher Covington
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2014-07-09 18:22 UTC (permalink / raw)
To: linux-arm-kernel
Andy pointed out that binutils generates additional sections in the vdso
image (e.g. section string table) which, if our .text section gets big
enough, could cross a page boundary and end up screwing up the location
where the kernel expects to put the data page.
This patch solves the issue in the same manner as x86_32, by moving the
data page before the code pages.
Cc: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/vdso.c | 34 +++++++++++++++++-----------------
arch/arm64/kernel/vdso/vdso.lds.S | 4 +---
2 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 60ae12087d9f..24f2e8c62479 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -121,8 +121,8 @@ static int __init vdso_init(void)
}
vdso_pages = (&vdso_end - &vdso_start) >> PAGE_SHIFT;
- pr_info("vdso: %ld pages (%ld code, %ld data) at base %p\n",
- vdso_pages + 1, vdso_pages, 1L, &vdso_start);
+ pr_info("vdso: %ld pages (%ld code @ %p, %ld data @ %p)\n",
+ vdso_pages + 1, vdso_pages, &vdso_start, 1L, vdso_data);
/* Allocate the vDSO pagelist, plus a page for the data. */
vdso_pagelist = kcalloc(vdso_pages + 1, sizeof(struct page *),
@@ -130,22 +130,22 @@ static int __init vdso_init(void)
if (vdso_pagelist == NULL)
return -ENOMEM;
+ /* Grab the vDSO data page. */
+ vdso_pagelist[0] = virt_to_page(vdso_data);
+
/* Grab the vDSO code pages. */
for (i = 0; i < vdso_pages; i++)
- vdso_pagelist[i] = virt_to_page(&vdso_start + i * PAGE_SIZE);
-
- /* Grab the vDSO data page. */
- vdso_pagelist[i] = virt_to_page(vdso_data);
+ vdso_pagelist[i + 1] = virt_to_page(&vdso_start + i * PAGE_SIZE);
/* Populate the special mapping structures */
vdso_spec[0] = (struct vm_special_mapping) {
- .name = "[vdso]",
+ .name = "[vvar]",
.pages = vdso_pagelist,
};
vdso_spec[1] = (struct vm_special_mapping) {
- .name = "[vvar]",
- .pages = vdso_pagelist + vdso_pages,
+ .name = "[vdso]",
+ .pages = &vdso_pagelist[1],
};
return 0;
@@ -169,22 +169,22 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
ret = ERR_PTR(vdso_base);
goto up_fail;
}
- mm->context.vdso = (void *)vdso_base;
-
- ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
- VM_READ|VM_EXEC|
- VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
+ ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
+ VM_READ|VM_MAYREAD,
&vdso_spec[0]);
if (IS_ERR(ret))
goto up_fail;
- vdso_base += vdso_text_len;
- ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
- VM_READ|VM_MAYREAD,
+ vdso_base += PAGE_SIZE;
+ mm->context.vdso = (void *)vdso_base;
+ ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
+ VM_READ|VM_EXEC|
+ VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
&vdso_spec[1]);
if (IS_ERR(ret))
goto up_fail;
+
up_write(&mm->mmap_sem);
return 0;
diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index 8154b8d1c826..beca249bc2f3 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -28,6 +28,7 @@ OUTPUT_ARCH(aarch64)
SECTIONS
{
+ PROVIDE(_vdso_data = . - PAGE_SIZE);
. = VDSO_LBASE + SIZEOF_HEADERS;
.hash : { *(.hash) } :text
@@ -57,9 +58,6 @@ SECTIONS
_end = .;
PROVIDE(end = .);
- . = ALIGN(PAGE_SIZE);
- PROVIDE(_vdso_data = .);
-
/DISCARD/ : {
*(.note.GNU-stack)
*(.data .data.* .gnu.linkonce.d.* .sdata*)
--
2.0.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 0/3] Bunch of vdso cleanups following feedback from x86
2014-07-09 18:22 [PATCH 0/3] Bunch of vdso cleanups following feedback from x86 Will Deacon
` (2 preceding siblings ...)
2014-07-09 18:22 ` [PATCH 3/3] arm64: vdso: move data page before code pages Will Deacon
@ 2014-07-24 18:20 ` Christopher Covington
3 siblings, 0 replies; 7+ messages in thread
From: Christopher Covington @ 2014-07-24 18:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Will,
On 07/09/2014 02:22 PM, Will Deacon wrote:
> Hi all,
>
> Following useful discussions with Andy (thanks!), I've put together the
> following (lightly tested) series for arm64 which attempts to address
> issues with our current vdso code.
>
> Please take a look. Cheers,
With appropriate updates to Checkpoint and Restore In Userspace (CRIU) [1] and
some finishing touches to its AArch64 port that I hope to publish very soon, I
see no regressions running its VDSO test cases, which exercise basic
discovery, use, and remapping.
Tested-by: Christopher Covington <cov@codeaurora.org>
1. http://lists.openvz.org/pipermail/criu/2014-July/015138.html
Christopher
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.
^ permalink raw reply [flat|nested] 7+ messages in thread