linux-kernel - Re: [PATCH 1/3] arm64: Introduce a way to disable the 32bit vdso

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]

Message-ID: <20200702101819.GB15391@C02TD0UTHF1T.local>
Date:   Thu, 2 Jul 2020 11:18:19 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: [PATCH 1/3] arm64: Introduce a way to disable the 32bit vdso

On Wed, Jul 01, 2020 at 05:18:22PM +0100, Marc Zyngier wrote:
> We have a class of errata (grouped under the ARM64_WORKAROUND_1418040
> banner) that force the trapping of counter access from 32bit EL0.
> 
> We would normally disable the whole vdso for such defect, except that
> it would disable it for 64bit userspace as well, which is a shame.
> 
> Instead, add a new vdso_clock_mode, which signals that the vdso
> isn't usable for compat tasks.  This gets checked in the new
> vdso_clocksource_ok() helper, now provided for the 32bit vdso.
> 
> Signed-off-by: Marc Zyngier <maz@...nel.org>

As-is this looks sound to me. It's not entirely clear to me how this
will compose with VDSO_CLOCKMODE_TIMENS, but given this series is fixing
an existing brokenness I think this has higher priority.

Minor comment below, but regardless:

Acked-by: Mark Rutland <mark.rutland@....com>

> ---
>  arch/arm64/include/asm/vdso/clocksource.h         | 7 +++++--
>  arch/arm64/include/asm/vdso/compat_gettimeofday.h | 8 +++++++-
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/vdso/clocksource.h b/arch/arm64/include/asm/vdso/clocksource.h
> index df6ea65c1dec..b054d9febfb5 100644
> --- a/arch/arm64/include/asm/vdso/clocksource.h
> +++ b/arch/arm64/include/asm/vdso/clocksource.h
> @@ -2,7 +2,10 @@
>  #ifndef __ASM_VDSOCLOCKSOURCE_H
>  #define __ASM_VDSOCLOCKSOURCE_H
>  
> -#define VDSO_ARCH_CLOCKMODES	\
> -	VDSO_CLOCKMODE_ARCHTIMER
> +#define VDSO_ARCH_CLOCKMODES					\
> +	/* vdso clocksource for both 32 and 64bit tasks */	\
> +	VDSO_CLOCKMODE_ARCHTIMER,				\
> +	/* vdso clocksource for 64bit tasks only */		\
> +	VDSO_CLOCKMODE_ARCHTIMER_NOCOMPAT
>  
>  #endif
> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index b6907ae78e53..9a625e8947ff 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -111,7 +111,7 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode)
>  	 * update. Return something. Core will do another round and then
>  	 * see the mode change and fallback to the syscall.
>  	 */
> -	if (clock_mode == VDSO_CLOCKMODE_NONE)
> +	if (clock_mode != VDSO_CLOCKMODE_ARCHTIMER)
>  		return 0;
>  
>  	/*
> @@ -152,6 +152,12 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
>  	return ret;
>  }
>  
> +static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
> +{
> +	return vd->clock_mode == VDSO_CLOCKMODE_ARCHTIMER;
> +}
> +#define vdso_clocksource_ok	vdso_clocksource_ok

Existing issue, but it's a shame this doesn't take the clock mode
directly, then we wouldn't have to duplicate the conditions (as above,
inverted).

Mark.