lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Feb 2020 12:03:27 +0000
From:   Vincenzo Frascino <vincenzo.frascino@....com>
To:     Andrei Vagin <avagin@...il.com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Dmitry Safonov <dima@...sta.com>
Subject: Re: [PATCH 3/5] arm64/vdso: Add time napespace page

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 <avagin@...il.com>
> ---
>  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

Download attachment "pEpkey.asc" of type "application/pgp-keys" (14072 bytes)

Powered by blists - more mailing lists