Allocate the time namespace page among VVAR pages and add the logic
to handle faults on VVAR properly.
If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page. That page has vdso_data->seq
set to 1 to enforce the slow path and vdso_data->clock_mode set to
VCLOCK_TIMENS to enforce the time namespace handling path.
The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
update of the VDSO data is in progress, is not really affecting regular
tasks which are not part of a time namespace as the task is spin waiting
for the update to finish and vdso_data->seq to become even again.
If a time namespace task hits that code path, it invokes the corresponding
time getter function which retrieves the real VVAR page, reads host time
and then adds the offset for the requested clock which is stored in the
special VVAR page.
Cc: Vincenzo Frascino <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Andrei Vagin (5):
arm64/vdso: use the fault callback to map vvar pages
arm64/vdso: Zap vvar pages when switching to a time namespace
arm64/vdso: Add time napespace page
arm64/vdso: Handle faults on timens page
arm64/vdso: Restrict splitting VVAR VMA
arch/arm64/Kconfig | 1 +
.../include/asm/vdso/compat_gettimeofday.h | 11 ++
arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++
arch/arm64/kernel/vdso.c | 134 ++++++++++++++++--
arch/arm64/kernel/vdso/vdso.lds.S | 3 +-
arch/arm64/kernel/vdso32/vdso.lds.S | 3 +-
include/vdso/datapage.h | 1 +
7 files changed, 147 insertions(+), 14 deletions(-)
--
2.24.1
Allocate the time namespace page among VVAR pages. Provide
__arch_get_timens_vdso_data() helper for VDSO code to get the
code-relative position of VVARs on that special page.
If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page. That page has vdso_data->seq
set to 1 to enforce the slow path and vdso_data->clock_mode set to
VCLOCK_TIMENS to enforce the time namespace handling path.
The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
update of the VDSO data is in progress, is not really affecting regular
tasks which are not part of a time namespace as the task is spin waiting
for the update to finish and vdso_data->seq to become even again.
If a time namespace task hits that code path, it invokes the corresponding
time getter function which retrieves the real VVAR page, reads host time
and then adds the offset for the requested clock which is stored in the
special VVAR page.
Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/Kconfig | 1 +
.../arm64/include/asm/vdso/compat_gettimeofday.h | 11 +++++++++++
arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++++++++
arch/arm64/kernel/vdso.c | 16 +++++++++++++---
arch/arm64/kernel/vdso/vdso.lds.S | 3 ++-
arch/arm64/kernel/vdso32/vdso.lds.S | 3 ++-
include/vdso/datapage.h | 1 +
7 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e688dfad0b72..a671c2e36e5f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -109,6 +109,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select GENERIC_GETTIMEOFDAY
+ select GENERIC_VDSO_TIME_NS
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_PCI
diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 537b1e695365..30a674f598c7 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -161,6 +161,17 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
return ret;
}
+#ifdef CONFIG_TIME_NS
+static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+{
+ const struct vdso_data *ret;
+
+ asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
+
+ return ret;
+}
+#endif
+
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index b08f476b72b4..aa38e80dfbc4 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -98,6 +98,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
return _vdso_data;
}
+#ifdef CONFIG_TIME_NS
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(void)
+{
+ return _timens_data;
+}
+#endif
+
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 5ef808ddf08c..bc93e26ae485 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -46,6 +46,10 @@ enum arch_vdso_type {
#define VDSO_TYPES (ARM64_VDSO + 1)
#endif /* CONFIG_COMPAT_VDSO */
+#define VVAR_DATA_PAGE_OFFSET 0
+#define VVAR_TIMENS_PAGE_OFFSET 1
+#define VVAR_NR_PAGES 2
+
struct __vdso_abi {
const char *name;
const char *vdso_code_start;
@@ -81,6 +85,12 @@ static union {
} vdso_data_store __page_aligned_data;
struct vdso_data *vdso_data = vdso_data_store.data;
+
+struct vdso_data *arch_get_vdso_data(void *vvar_page)
+{
+ return (struct vdso_data *)(vvar_page);
+}
+
static int __vdso_remap(enum arch_vdso_type arch_index,
const struct vm_special_mapping *sm,
struct vm_area_struct *new_vma)
@@ -182,7 +192,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
vdso_text_len = vdso_lookup[arch_index].vdso_pages << PAGE_SHIFT;
/* Be sure to map the data page */
- vdso_mapping_len = vdso_text_len + PAGE_SIZE;
+ vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
if (IS_ERR_VALUE(vdso_base)) {
@@ -190,13 +200,13 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
goto up_fail;
}
- ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
+ ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
VM_READ|VM_MAYREAD|VM_PFNMAP,
vdso_lookup[arch_index].dm);
if (IS_ERR(ret))
goto up_fail;
- vdso_base += PAGE_SIZE;
+ vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
mm->context.vdso = (void *)vdso_base;
ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
VM_READ|VM_EXEC|
diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index 7ad2d3a0cd48..a90b7d14e990 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -17,7 +17,8 @@ OUTPUT_ARCH(aarch64)
SECTIONS
{
- PROVIDE(_vdso_data = . - PAGE_SIZE);
+ PROVIDE(_vdso_data = . - 2 * PAGE_SIZE);
+ PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
. = VDSO_LBASE + SIZEOF_HEADERS;
.hash : { *(.hash) } :text
diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
index a3944927eaeb..3e432b536e53 100644
--- a/arch/arm64/kernel/vdso32/vdso.lds.S
+++ b/arch/arm64/kernel/vdso32/vdso.lds.S
@@ -17,7 +17,8 @@ OUTPUT_ARCH(arm)
SECTIONS
{
- PROVIDE_HIDDEN(_vdso_data = . - PAGE_SIZE);
+ PROVIDE_HIDDEN(_vdso_data = . - 2 * PAGE_SIZE);
+ PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
. = VDSO_LBASE + SIZEOF_HEADERS;
.hash : { *(.hash) } :text
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index c5f347cc5e55..57eec6caca69 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -100,6 +100,7 @@ struct vdso_data {
* relocation, and this is what we need.
*/
extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
+extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
#endif /* !__ASSEMBLY__ */
--
2.24.1
The VVAR page layout depends on whether a task belongs to the root or
non-root time namespace. Whenever a task changes its namespace, the VVAR
page tables are cleared and then they will be re-faulted with a
corresponding layout.
Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/kernel/vdso.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index c4b4758eaf0b..5ef808ddf08c 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -131,6 +131,38 @@ static int __vdso_init(enum arch_vdso_type arch_index)
return 0;
}
+#ifdef CONFIG_TIME_NS
+/*
+ * The vvar page layout depends on whether a task belongs to the root or
+ * non-root time namespace. Whenever a task changes its namespace, the VVAR
+ * page tables are cleared and then they will re-faulted with a
+ * corresponding layout.
+ * See also the comment near timens_setup_vdso_data() for details.
+ */
+int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
+{
+ struct mm_struct *mm = task->mm;
+ struct vm_area_struct *vma;
+
+ if (down_write_killable(&mm->mmap_sem))
+ return -EINTR;
+
+ for (vma = mm->mmap; vma; vma = vma->vm_next) {
+ unsigned long size = vma->vm_end - vma->vm_start;
+
+ if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO].dm))
+ zap_page_range(vma, vma->vm_start, size);
+#ifdef CONFIG_COMPAT_VDSO
+ if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO32].dm))
+ zap_page_range(vma, vma->vm_start, size);
+#endif
+ }
+
+ up_write(&mm->mmap_sem);
+ return 0;
+}
+#endif
+
static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
struct vm_area_struct *vma, struct vm_fault *vmf)
{
--
2.24.1
This is required to support time namespaces where a time namespace data
page is different for each namespace.
Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/kernel/vdso.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 354b11e27c07..c4b4758eaf0b 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -114,28 +114,32 @@ static int __vdso_init(enum arch_vdso_type arch_index)
PAGE_SHIFT;
/* Allocate the vDSO pagelist, plus a page for the data. */
- vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages + 1,
+ vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages,
sizeof(struct page *),
GFP_KERNEL);
if (vdso_pagelist == NULL)
return -ENOMEM;
- /* Grab the vDSO data page. */
- vdso_pagelist[0] = phys_to_page(__pa_symbol(vdso_data));
-
-
/* Grab the vDSO code pages. */
pfn = sym_to_pfn(vdso_lookup[arch_index].vdso_code_start);
for (i = 0; i < vdso_lookup[arch_index].vdso_pages; i++)
- vdso_pagelist[i + 1] = pfn_to_page(pfn + i);
+ vdso_pagelist[i] = pfn_to_page(pfn + i);
- vdso_lookup[arch_index].dm->pages = &vdso_pagelist[0];
- vdso_lookup[arch_index].cm->pages = &vdso_pagelist[1];
+ vdso_lookup[arch_index].cm->pages = vdso_pagelist;
return 0;
}
+static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ if (vmf->pgoff == 0)
+ return vmf_insert_pfn(vma, vmf->address,
+ sym_to_pfn(vdso_data));
+ return VM_FAULT_SIGBUS;
+}
+
static int __setup_additional_pages(enum arch_vdso_type arch_index,
struct mm_struct *mm,
struct linux_binprm *bprm,
@@ -155,7 +159,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
}
ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
- VM_READ|VM_MAYREAD,
+ VM_READ|VM_MAYREAD|VM_PFNMAP,
vdso_lookup[arch_index].dm);
if (IS_ERR(ret))
goto up_fail;
@@ -206,6 +210,8 @@ static int aarch32_vdso_mremap(const struct vm_special_mapping *sm,
#define C_SIGPAGE 1
#define C_PAGES (C_SIGPAGE + 1)
#endif /* CONFIG_COMPAT_VDSO */
+static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf);
static struct page *aarch32_vdso_pages[C_PAGES] __ro_after_init;
static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
{
@@ -215,6 +221,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
#ifdef CONFIG_COMPAT_VDSO
{
.name = "[vvar]",
+ .fault = vvar_fault,
},
{
.name = "[vdso]",
@@ -396,6 +403,7 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
{
.name = "[vvar]",
+ .fault = vvar_fault,
},
{
.name = "[vdso]",
--
2.24.1
Forbid splitting VVAR VMA resulting in a stricter ABI and reducing the
amount of corner-cases to consider while working further on VDSO time
namespace support.
As the offset from timens to VVAR page is computed compile-time, the pages
in VVAR should stay together and not being partically mremap()'ed.
Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/kernel/vdso.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 2e553468b183..e6ebdc184c1e 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -229,6 +229,17 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
return vmf_insert_pfn(vma, vmf->address, pfn);
}
+static int vvar_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;
+
+ if (new_size != VVAR_NR_PAGES * PAGE_SIZE)
+ return -EINVAL;
+
+ return 0;
+}
+
static int __setup_additional_pages(enum arch_vdso_type arch_index,
struct mm_struct *mm,
struct linux_binprm *bprm,
@@ -311,6 +322,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
{
.name = "[vvar]",
.fault = vvar_fault,
+ .mremap = vvar_mremap,
},
{
.name = "[vdso]",
@@ -493,6 +505,7 @@ static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
{
.name = "[vvar]",
.fault = vvar_fault,
+ .mremap = vvar_mremap,
},
{
.name = "[vdso]",
--
2.24.1
If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page.
Signed-off-by: Andrei Vagin <[email protected]>
---
arch/arm64/kernel/vdso.c | 55 +++++++++++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index bc93e26ae485..2e553468b183 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -23,6 +23,7 @@
#include <vdso/datapage.h>
#include <vdso/helpers.h>
#include <vdso/vsyscall.h>
+#include <linux/time_namespace.h>
#include <asm/cacheflush.h>
#include <asm/signal32.h>
@@ -171,15 +172,61 @@ int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
up_write(&mm->mmap_sem);
return 0;
}
+
+static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ if (likely(vma->vm_mm == current->mm))
+ return current->nsproxy->time_ns->vvar_page;
+
+ /*
+ * VM_PFNMAP | VM_IO protect .fault() handler from being called
+ * through interfaces like /proc/$pid/mem or
+ * process_vm_{readv,writev}() as long as there's no .access()
+ * in special_mapping_vmops().
+ * For more details check_vma_flags() and __access_remote_vm()
+ */
+
+ WARN(1, "vvar_page accessed remotely");
+
+ return NULL;
+}
+#else
+static inline struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ return NULL;
+}
#endif
static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
struct vm_area_struct *vma, struct vm_fault *vmf)
{
- if (vmf->pgoff == 0)
- return vmf_insert_pfn(vma, vmf->address,
- sym_to_pfn(vdso_data));
- return VM_FAULT_SIGBUS;
+ struct page *timens_page = find_timens_vvar_page(vma);
+ unsigned long pfn;
+
+ switch (vmf->pgoff) {
+ case VVAR_DATA_PAGE_OFFSET:
+ if (timens_page)
+ pfn = page_to_pfn(timens_page);
+ else
+ pfn = sym_to_pfn(vdso_data);
+ break;
+ case VVAR_TIMENS_PAGE_OFFSET:
+ /*
+ * If a task belongs to a time namespace then a namespace
+ * specific VVAR is mapped with the VVAR_DATA_PAGE_OFFSET and
+ * the real VVAR page is mapped with the VVAR_TIMENS_PAGE_OFFSET
+ * offset.
+ * See also the comment near timens_setup_vdso_data().
+ */
+ if (!timens_page)
+ return VM_FAULT_SIGBUS;
+ pfn = sym_to_pfn(vdso_data);
+ break;
+ default:
+ return VM_FAULT_SIGBUS;
+ }
+
+ return vmf_insert_pfn(vma, vmf->address, pfn);
}
static int __setup_additional_pages(enum arch_vdso_type arch_index,
--
2.24.1
Vincenzo, could you take a look this patchset?
On Tue, Feb 04, 2020 at 09:59:08AM -0800, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages and add the logic
> to handle faults on VVAR properly.
>
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
>
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
>
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
>
> Cc: Vincenzo Frascino <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Dmitry Safonov <[email protected]>
>
> Andrei Vagin (5):
> arm64/vdso: use the fault callback to map vvar pages
> arm64/vdso: Zap vvar pages when switching to a time namespace
> arm64/vdso: Add time napespace page
> arm64/vdso: Handle faults on timens page
> arm64/vdso: Restrict splitting VVAR VMA
>
> arch/arm64/Kconfig | 1 +
> .../include/asm/vdso/compat_gettimeofday.h | 11 ++
> arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++
> arch/arm64/kernel/vdso.c | 134 ++++++++++++++++--
> arch/arm64/kernel/vdso/vdso.lds.S | 3 +-
> arch/arm64/kernel/vdso32/vdso.lds.S | 3 +-
> include/vdso/datapage.h | 1 +
> 7 files changed, 147 insertions(+), 14 deletions(-)
>
> --
> 2.24.1
>
Hi Andrei,
On 17/02/2020 15:34, Andrei Vagin wrote:
> Vincenzo, could you take a look this patchset?
>
Apologize for not getting to this yet. I will have a look at your patches during
this week.
> On Tue, Feb 04, 2020 at 09:59:08AM -0800, Andrei Vagin wrote:
>> Allocate the time namespace page among VVAR pages and add the logic
>> to handle faults on VVAR properly.
>>
>> If a task belongs to a time namespace then the VVAR page which contains
>> the system wide VDSO data is replaced with a namespace specific page
>> which has the same layout as the VVAR page. That page has vdso_data->seq
>> set to 1 to enforce the slow path and vdso_data->clock_mode set to
>> VCLOCK_TIMENS to enforce the time namespace handling path.
>>
>> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
>> update of the VDSO data is in progress, is not really affecting regular
>> tasks which are not part of a time namespace as the task is spin waiting
>> for the update to finish and vdso_data->seq to become even again.
>>
>> If a time namespace task hits that code path, it invokes the corresponding
>> time getter function which retrieves the real VVAR page, reads host time
>> and then adds the offset for the requested clock which is stored in the
>> special VVAR page.
>>
>> Cc: Vincenzo Frascino <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Dmitry Safonov <[email protected]>
>>
>> Andrei Vagin (5):
>> arm64/vdso: use the fault callback to map vvar pages
>> arm64/vdso: Zap vvar pages when switching to a time namespace
>> arm64/vdso: Add time napespace page
>> arm64/vdso: Handle faults on timens page
>> arm64/vdso: Restrict splitting VVAR VMA
>>
>> arch/arm64/Kconfig | 1 +
>> .../include/asm/vdso/compat_gettimeofday.h | 11 ++
>> arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++
>> arch/arm64/kernel/vdso.c | 134 ++++++++++++++++--
>> arch/arm64/kernel/vdso/vdso.lds.S | 3 +-
>> arch/arm64/kernel/vdso32/vdso.lds.S | 3 +-
>> include/vdso/datapage.h | 1 +
>> 7 files changed, 147 insertions(+), 14 deletions(-)
>>
>> --
>> 2.24.1
>>
--
Regards,
Vincenzo
Hi Andrei,
On 04/02/2020 17:59, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages. Provide
> __arch_get_timens_vdso_data() helper for VDSO code to get the
> code-relative position of VVARs on that special page.
>
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
>
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
>
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
>
> Signed-off-by: Andrei Vagin <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> .../arm64/include/asm/vdso/compat_gettimeofday.h | 11 +++++++++++
> arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++++++++
> arch/arm64/kernel/vdso.c | 16 +++++++++++++---
> arch/arm64/kernel/vdso/vdso.lds.S | 3 ++-
> arch/arm64/kernel/vdso32/vdso.lds.S | 3 ++-
> include/vdso/datapage.h | 1 +
> 7 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index e688dfad0b72..a671c2e36e5f 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -109,6 +109,7 @@ config ARM64
> select GENERIC_STRNLEN_USER
> select GENERIC_TIME_VSYSCALL
> select GENERIC_GETTIMEOFDAY
> + select GENERIC_VDSO_TIME_NS
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_PCI
Could you please add the selection in a separate patch at the end of this series?
> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index 537b1e695365..30a674f598c7 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -161,6 +161,17 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
> return ret;
> }
>
> +#ifdef CONFIG_TIME_NS
> +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> + const struct vdso_data *ret;
> +
> + asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> +
> + return ret;
> +}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
> index b08f476b72b4..aa38e80dfbc4 100644
> --- a/arch/arm64/include/asm/vdso/gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/gettimeofday.h
> @@ -98,6 +98,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
> return _vdso_data;
> }
>
> +#ifdef CONFIG_TIME_NS
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> + return _timens_data;
> +}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
>
> #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 5ef808ddf08c..bc93e26ae485 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -46,6 +46,10 @@ enum arch_vdso_type {
> #define VDSO_TYPES (ARM64_VDSO + 1)
> #endif /* CONFIG_COMPAT_VDSO */
>
> +#define VVAR_DATA_PAGE_OFFSET 0
> +#define VVAR_TIMENS_PAGE_OFFSET 1
> +#define VVAR_NR_PAGES 2
> +
Maybe we could change this in an enumeration. Something like:
enum vvar_pages {
VVAR_DATA_PAGE_OFFSET = 0,
#ifdef CONFIG_TIME_NS
VVAR_TIMENS_PAGE_OFFSET = 1,
#endif /* CONFIG_TIME_NS */
VVAR_NR_PAGES = __VDSO_PAGES,
(look at the change suggested below for vdso.h)
};
> struct __vdso_abi {
> const char *name;
> const char *vdso_code_start;
> @@ -81,6 +85,12 @@ static union {
> } vdso_data_store __page_aligned_data;
> struct vdso_data *vdso_data = vdso_data_store.data;
>
> +
> +struct vdso_data *arch_get_vdso_data(void *vvar_page)
> +{
> + return (struct vdso_data *)(vvar_page);
> +}
> +
> static int __vdso_remap(enum arch_vdso_type arch_index,
> const struct vm_special_mapping *sm,
> struct vm_area_struct *new_vma)
> @@ -182,7 +192,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
>
> vdso_text_len = vdso_lookup[arch_index].vdso_pages << PAGE_SHIFT;
> /* Be sure to map the data page */
> - vdso_mapping_len = vdso_text_len + PAGE_SIZE;
> + vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
>
> vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
> if (IS_ERR_VALUE(vdso_base)) {
> @@ -190,13 +200,13 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
> goto up_fail;
> }
>
> - ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
> + ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
> VM_READ|VM_MAYREAD|VM_PFNMAP,
> vdso_lookup[arch_index].dm);
> if (IS_ERR(ret))
> goto up_fail;
>
> - vdso_base += PAGE_SIZE;
> + vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
> mm->context.vdso = (void *)vdso_base;
> ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
> VM_READ|VM_EXEC|
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index 7ad2d3a0cd48..a90b7d14e990 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -17,7 +17,8 @@ OUTPUT_ARCH(aarch64)
>
> SECTIONS
> {
> - PROVIDE(_vdso_data = . - PAGE_SIZE);
> + PROVIDE(_vdso_data = . - 2 * PAGE_SIZE);
Maybe we could have some conditional definition on CONFIG_TIME_NS in vdso.h:
#ifdef CONFIG_TIME_NS
#define __VVAR_PAGES 2
#else
#define __VVAR_PAGES 1
#endif
and then here:
PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
just to not forget what "2" stands for. What do you think?
> + PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
> . = VDSO_LBASE + SIZEOF_HEADERS;
>
> .hash : { *(.hash) } :text
> diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
> index a3944927eaeb..3e432b536e53 100644
> --- a/arch/arm64/kernel/vdso32/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso32/vdso.lds.S
> @@ -17,7 +17,8 @@ OUTPUT_ARCH(arm)
>
> SECTIONS
> {
> - PROVIDE_HIDDEN(_vdso_data = . - PAGE_SIZE);
> + PROVIDE_HIDDEN(_vdso_data = . - 2 * PAGE_SIZE);
Ditto.
> + PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
> . = VDSO_LBASE + SIZEOF_HEADERS;
>
> .hash : { *(.hash) } :text
> diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
> index c5f347cc5e55..57eec6caca69 100644
> --- a/include/vdso/datapage.h
> +++ b/include/vdso/datapage.h
> @@ -100,6 +100,7 @@ struct vdso_data {
> * relocation, and this is what we need.
> */
> extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
> +extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
>
> #endif /* !__ASSEMBLY__ */
>
>
--
Regards,
Vincenzo
Hi Andrei,
On 04/02/2020 17:59, Andrei Vagin wrote:
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page.
>
> Signed-off-by: Andrei Vagin <[email protected]>
> ---
> arch/arm64/kernel/vdso.c | 55 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index bc93e26ae485..2e553468b183 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -23,6 +23,7 @@
> #include <vdso/datapage.h>
> #include <vdso/helpers.h>
> #include <vdso/vsyscall.h>
> +#include <linux/time_namespace.h>
>
Nit: Could you please group this with the linux/ headers and keep the
alphabetical order.
> #include <asm/cacheflush.h>
> #include <asm/signal32.h>
> @@ -171,15 +172,61 @@ int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
> up_write(&mm->mmap_sem);
> return 0;
> }
> +
> +static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
> +{
> + if (likely(vma->vm_mm == current->mm))
> + return current->nsproxy->time_ns->vvar_page;
> +
> + /*
> + * VM_PFNMAP | VM_IO protect .fault() handler from being called
> + * through interfaces like /proc/$pid/mem or
> + * process_vm_{readv,writev}() as long as there's no .access()
> + * in special_mapping_vmops().
> + * For more details check_vma_flags() and __access_remote_vm()
> + */
> +
> + WARN(1, "vvar_page accessed remotely");
> +
> + return NULL;
> +}
> +#else
> +static inline struct page *find_timens_vvar_page(struct vm_area_struct *vma)
> +{
> + return NULL;
> +}
> #endif
>
> static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
> struct vm_area_struct *vma, struct vm_fault *vmf)
> {
> - if (vmf->pgoff == 0)
> - return vmf_insert_pfn(vma, vmf->address,
> - sym_to_pfn(vdso_data));
> - return VM_FAULT_SIGBUS;
> + struct page *timens_page = find_timens_vvar_page(vma);
> + unsigned long pfn;
> +
> + switch (vmf->pgoff) {
> + case VVAR_DATA_PAGE_OFFSET:
> + if (timens_page)
> + pfn = page_to_pfn(timens_page);
> + else
> + pfn = sym_to_pfn(vdso_data);
> + break;
> + case VVAR_TIMENS_PAGE_OFFSET:
> + /*
> + * If a task belongs to a time namespace then a namespace
> + * specific VVAR is mapped with the VVAR_DATA_PAGE_OFFSET and
> + * the real VVAR page is mapped with the VVAR_TIMENS_PAGE_OFFSET
> + * offset.
> + * See also the comment near timens_setup_vdso_data().
> + */
> + if (!timens_page)
> + return VM_FAULT_SIGBUS;
> + pfn = sym_to_pfn(vdso_data);
> + break;
> + default:
> + return VM_FAULT_SIGBUS;
> + }
> +
> + return vmf_insert_pfn(vma, vmf->address, pfn);
> }
>
> static int __setup_additional_pages(enum arch_vdso_type arch_index,
>
--
Regards,
Vincenzo
Hi Andrei,
On 04/02/2020 17:59, Andrei Vagin wrote:
> Forbid splitting VVAR VMA resulting in a stricter ABI and reducing the
> amount of corner-cases to consider while working further on VDSO time
> namespace support.
>
> As the offset from timens to VVAR page is computed compile-time, the pages
> in VVAR should stay together and not being partically mremap()'ed.
>
I agree on the concept, but why do we need to redefine mremap?
special_mapping_mremap() (mm/mmap.c +3317) seems doing already the same thing if
we leave mremap == NULL as is.
> Signed-off-by: Andrei Vagin <[email protected]>
> ---
> arch/arm64/kernel/vdso.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 2e553468b183..e6ebdc184c1e 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -229,6 +229,17 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
> return vmf_insert_pfn(vma, vmf->address, pfn);
> }
>
> +static int vvar_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;
> +
> + if (new_size != VVAR_NR_PAGES * PAGE_SIZE)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int __setup_additional_pages(enum arch_vdso_type arch_index,
> struct mm_struct *mm,
> struct linux_binprm *bprm,
> @@ -311,6 +322,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
> {
> .name = "[vvar]",
> .fault = vvar_fault,
> + .mremap = vvar_mremap,
> },
> {
> .name = "[vdso]",
> @@ -493,6 +505,7 @@ static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
> {
> .name = "[vvar]",
> .fault = vvar_fault,
> + .mremap = vvar_mremap,
> },
> {
> .name = "[vdso]",
>
--
Regards,
Vincenzo
Hi Andrei,
On 04/02/2020 17:59, Andrei Vagin wrote:
> This is required to support time namespaces where a time namespace data
> page is different for each namespace.
>
> Signed-off-by: Andrei Vagin <[email protected]>
> ---
[...]
> @@ -206,6 +210,8 @@ static int aarch32_vdso_mremap(const struct vm_special_mapping *sm,
> #define C_SIGPAGE 1
> #define C_PAGES (C_SIGPAGE + 1)
> #endif /* CONFIG_COMPAT_VDSO */
> +static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
> + struct vm_area_struct *vma, struct vm_fault *vmf);
I think you forgot to remove this one :)
[...]
--
Regards,
Vincenzo
Hi Andrei,
On 04/02/2020 17:59, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages and add the logic
> to handle faults on VVAR properly.
>
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
>
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
>
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
>
Thank you for adding the arm64 support of time namespaces. Overall it looks fine
to me even if I have few comments. I will test it in the coming days just to
make sure I did not miss something major. I will keep you updated on the results.
> Cc: Vincenzo Frascino <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Dmitry Safonov <[email protected]>
>
> Andrei Vagin (5):
> arm64/vdso: use the fault callback to map vvar pages
> arm64/vdso: Zap vvar pages when switching to a time namespace
> arm64/vdso: Add time napespace page
> arm64/vdso: Handle faults on timens page
> arm64/vdso: Restrict splitting VVAR VMA
>
> arch/arm64/Kconfig | 1 +
> .../include/asm/vdso/compat_gettimeofday.h | 11 ++
> arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++
> arch/arm64/kernel/vdso.c | 134 ++++++++++++++++--
> arch/arm64/kernel/vdso/vdso.lds.S | 3 +-
> arch/arm64/kernel/vdso32/vdso.lds.S | 3 +-
> include/vdso/datapage.h | 1 +
> 7 files changed, 147 insertions(+), 14 deletions(-)
>
--
Regards,
Vincenzo
On Thu, Feb 20, 2020 at 12:40:47PM +0000, Vincenzo Frascino wrote:
> Hi Andrei,
>
> On 04/02/2020 17:59, Andrei Vagin wrote:
> > Allocate the time namespace page among VVAR pages and add the logic
> > to handle faults on VVAR properly.
> >
> > If a task belongs to a time namespace then the VVAR page which contains
> > the system wide VDSO data is replaced with a namespace specific page
> > which has the same layout as the VVAR page. That page has vdso_data->seq
> > set to 1 to enforce the slow path and vdso_data->clock_mode set to
> > VCLOCK_TIMENS to enforce the time namespace handling path.
> >
> > The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> > update of the VDSO data is in progress, is not really affecting regular
> > tasks which are not part of a time namespace as the task is spin waiting
> > for the update to finish and vdso_data->seq to become even again.
> >
> > If a time namespace task hits that code path, it invokes the corresponding
> > time getter function which retrieves the real VVAR page, reads host time
> > and then adds the offset for the requested clock which is stored in the
> > special VVAR page.
> >
>
> Thank you for adding the arm64 support of time namespaces. Overall it looks fine
> to me even if I have few comments. I will test it in the coming days just to
> make sure I did not miss something major. I will keep you updated on the results.
Thank you for the review. All comments look reasonable and I will
address them and post a second version.
Thanks,
Andrei
On Thu, Feb 20, 2020 at 12:22:52PM +0000, Vincenzo Frascino wrote:
> Hi Andrei,
>
> On 04/02/2020 17:59, Andrei Vagin wrote:
> > Forbid splitting VVAR VMA resulting in a stricter ABI and reducing the
> > amount of corner-cases to consider while working further on VDSO time
> > namespace support.
> >
> > As the offset from timens to VVAR page is computed compile-time, the pages
> > in VVAR should stay together and not being partically mremap()'ed.
> >
>
> I agree on the concept, but why do we need to redefine mremap?
> special_mapping_mremap() (mm/mmap.c +3317) seems doing already the same thing if
> we leave mremap == NULL as is.
>
Hmmm. I have read the code of special_mapping_mremap() and I don't see where
it restricts splitting the vvar mapping.
Here is the code what I see in the source:
static int special_mapping_mremap(struct vm_area_struct *new_vma)
{
struct vm_special_mapping *sm = new_vma->vm_private_data;
if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
return -EFAULT;
if (sm->mremap)
return sm->mremap(sm, new_vma);
return 0;
}
And I have checked that without this patch, I can remap only one page of
the vvar mapping.
Thanks,
Andrei
Hi Andrei,
On 2/23/20 11:30 PM, Andrei Vagin wrote:
[...]
>
> Hmmm. I have read the code of special_mapping_mremap() and I don't see where
> it restricts splitting the vvar mapping.
>
> Here is the code what I see in the source:
>
> static int special_mapping_mremap(struct vm_area_struct *new_vma)
> {
> struct vm_special_mapping *sm = new_vma->vm_private_data;
>
> if (WARN_ON_ONCE(current->mm != new_vma->vm_mm))
> return -EFAULT;
>
> if (sm->mremap)
> return sm->mremap(sm, new_vma);
>
> return 0;
> }
>
> And I have checked that without this patch, I can remap only one page of
> the vvar mapping.
>
I checked it a second time and I agree. The check on new_size is required in
this case.
> Thanks,
> Andrei
>
--
Regards,
Vincenzo