| 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
| ||
|
Message-ID: <54AFAB8C.5040803@arm.com>
Date: Fri, 09 Jan 2015 10:21:00 +0000
From: "Suzuki K. Poulose" <Suzuki.Poulose@....com>
To: Mark Rutland <mark.rutland@....com>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"yexl@...vell.com" <yexl@...vell.com>,
Catalin Marinas <Catalin.Marinas@....com>,
Will Deacon <Will.Deacon@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"leo.yan@...aro.org" <leo.yan@...aro.org>
Subject: Re: [PATCH 2/2] arm64: Emulate SETEND for AArch32 tasks
On 08/01/15 18:43, Mark Rutland wrote:
> Hi Suzuki,
>
> On Wed, Jan 07, 2015 at 04:16:45PM +0000, Suzuki K. Poulose wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@....com>
>>
>> Emulate deprecated 'setend' instruction for AArch32 bit tasks.
>>
>> setend [le/be] - Sets the endianness of EL0
>>
>> The hardware support for the instruction can be enabled by setting the
>> SCTLR_EL1.SED bit. Like the other emulated instructions it is controlled by
>> an entry in /proc/sys/abi/. For more information see :
>> Documentation/arm64/legacy_instructions.txt
>>
>> The instruction is emulated by setting/clearing the SPSR_EL1.E bit, which
>> will be reflected in the PSTATE.E in AArch32 context.
>
> A "fun" problem with emulating setend is that it will not always work
> unless we emulate the entire instruction set when userspace wants to be
> in an unsupported endianness.
>
> For implementations which are not bi-endian at EL0 (i.e. where
> ID_AA64MMFR0_EL1.BigEndEL0 == 0), SCTLR_EL1.E0E has a fixed value which
> we cannot change. The field names are misleading: in a BE-only system
> ID_AA64MMFR0_EL1.{BigEnd,BigEndEL0} == {0,0} and SCTLR_EL1.{EE,E0E} are
> fixed to {1,1}.
>
> I think we need to detect when EL0 has a fixed endianness such that we
> can treat the setend instruction as undefined. Otherwise we will
> silently fail to change EL0 endianness, advance the PC, and return to
> userspace in the wrong endianness, which will be very painful to debug.
> Userspace has the option of handling the resulting SIGILL in such cases.
You are right. I missed this scenario. To add to that things get
complicated when there are heterogeneous CPUs on the system that might
have differing bits for BigEndEL0. I will take a look at this one.
Thanks for pointing this out.
>
> That means we need to be able to fail to transition into INSN_EMULATE
> mode as we currently can when transitioning to INSN_HW.
>
>> This patch also restores the native endianness for the execution of signal
>> handlers, since the process could have changed the endianness.
>>
>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@....com>
>> ---
>> Documentation/arm64/legacy_instructions.txt | 5 ++
>> arch/arm64/Kconfig | 10 ++++
>> arch/arm64/include/asm/ptrace.h | 7 +++
>> arch/arm64/kernel/armv8_deprecated.c | 75 +++++++++++++++++++++++++++
>> arch/arm64/kernel/signal32.c | 5 +-
>> 5 files changed, 101 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/legacy_instructions.txt b/Documentation/arm64/legacy_instructions.txt
>> index a3b3da2..20e5621 100644
>> --- a/Documentation/arm64/legacy_instructions.txt
>> +++ b/Documentation/arm64/legacy_instructions.txt
>> @@ -43,3 +43,8 @@ Default: Undef (0)
>> Node: /proc/sys/abi/cp15_barrier
>> Status: Deprecated
>> Default: Emulate (1)
>> +
>> +* SETEND
>> +Node: /proc/sys/abi/setend
>> +Status: Deprecated
>> +Default: Emulate (1)
>
> Given we can't always emulate SETEND, should we document "Emulate where
> possible" or something to that effect?
>
Will fix it in the next revision.
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index b1f9a20..c6d1fd9 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -540,6 +540,16 @@ config CP15_BARRIER_EMULATION
>>
>> If unsure, say Y
>>
>> +config SETEND_EMULATION
>> + bool "Emulate SETEND instruction"
>> + help
>> + The SETEND instruction alters the data-endianness of the
>> + AArch32 EL0, and is deprecated in ARMv8.
>> +
>> + Say Y here to enable software emulation of the instruction
>> + for AArch32 userspace code.
>> +
>> + If unsure, say Y
>> endif
>>
>> endmenu
>> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
>> index 41ed9e1..d6dd9fd 100644
>> --- a/arch/arm64/include/asm/ptrace.h
>> +++ b/arch/arm64/include/asm/ptrace.h
>> @@ -58,6 +58,13 @@
>> #define COMPAT_PSR_Z_BIT 0x40000000
>> #define COMPAT_PSR_N_BIT 0x80000000
>> #define COMPAT_PSR_IT_MASK 0x0600fc00 /* If-Then execution state mask */
>> +
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +#define COMPAT_PSR_ENDSTATE COMPAT_PSR_E_BIT
>> +#else
>> +#define COMPAT_PSR_ENDSTATE 0
>> +#endif
>> +
>> /*
>> * These are 'magic' values for PTRACE_PEEKUSR that return info about where a
>> * process is located in memory.
>> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
>> index 9054447..dc91bac 100644
>> --- a/arch/arm64/kernel/armv8_deprecated.c
>> +++ b/arch/arm64/kernel/armv8_deprecated.c
>> @@ -477,6 +477,7 @@ ret:
>> }
>>
>> #define SCTLR_EL1_CP15BEN (1 << 5)
>> +#define SCTLR_EL1_SED (1 << 8)
>>
>> static inline void config_sctlr_el1(u32 clear, u32 set)
>> {
>> @@ -521,6 +522,77 @@ static struct insn_emulation_ops cp15_barrier_ops = {
>> .set_hw_mode = cp15_barrier_set_hw_mode,
>> };
>>
>> +static void setend_set_hw_mode(void *enable)
>> +{
>> + if (enable)
>> + config_sctlr_el1(SCTLR_EL1_SED, 0);
>> + else
>> + config_sctlr_el1(0, SCTLR_EL1_SED);
>> +}
>> +
>> +static int compat_setend_handler(struct pt_regs *regs, u32 endian)
>
> If we s/endian/big_endian/ here we can drop the comments within the
> function as the test will be easier to read. We could also s/u32/bool/.
>
OK
>> +{
>> + char insn[16] = "setend _e";
>
> Elsewhere (e.g. in cp15barrier_handler) we write these out in full
> rather than modifying a string on the stack. I think we should do the
> same here (we can change insn to a char * and assign the full relevant
> string in either branch).
>
> Doing so will mean grepping for '"setend be"' finds this function, which
> is handy.
>
Makes sense. Thanks for the review.
Thanks
Suzuki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists