Skip to contentSkip to conversation

arm64: Rework valid_user_regs

PATCH Superseded Mark Rutland 3 participants 7 messages Feb 16, 2016 – Mar 1, 2016

Subsystems (2)
arm64 ptrace support

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Mark Rutland

author reviewer
#0 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

We validate pstate using PSR_MODE32_BIT, which is part of the user-provided pstate (and cannot be trusted). Also, we conflate validation of AArch32 and AArch64 pstate values, making the code difficult to reason about.

Instead, validate the pstate value based on the associated task. The task may or may not be current (e.g. when using ptrace), so this must be passed explicitly by callers. To avoid circular header dependencies via sched.h, is_compat_task is pulled out of asm/ptrace.h.

To make the code possible to reason about, the AArch64 and AArch32 validation is split into separate functions. Software must respect the RES0 policy for SPSR bits, and thus the kernel mirrors the hardware policy (RAZ/WI) for bits as-yet unallocated. When these acquire an architected meaning writes may be permitted (potentially with additional validation).

Signed-off-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Dave Martin <[email protected]>
Cc James Morse <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/ptrace.h | 33 +++---------------
 arch/arm64/kernel/ptrace.c      | 77 +++++++++++++++++++++++++++++++++++++++--
 arch/arm64/kernel/signal.c      |  4 +--
 arch/arm64/kernel/signal32.c    |  2 +-
 4 files changed, 82 insertions(+), 34 deletions(-)

Since v1 [1]:
* Clear the full complement of RES0 bits
* Treat IL as RES0
* Treat SS as RES0 when not tracing
* renames: s/aa64/native, s/aa32/compat/

Since v2 [2]:
* Kill PSR_{f,s,x} and refer to fields explicitly
* Add COMPAT_PSR_GE_MASK

Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/406424.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/407386.html

arch/arm64/include/asm/ptrace.h +4 -29
index e9e5467..a307eb6 100644
@@ -58,6 +58,7 @@
 #define COMPAT_PSR_Z_BIT	0x40000000
 #define COMPAT_PSR_N_BIT	0x80000000
 #define COMPAT_PSR_IT_MASK	0x0600fc00	/* If-Then execution state mask */
+#define COMPAT_PSR_GE_MASK	0x000f0000
 
 #ifdef CONFIG_CPU_BIG_ENDIAN
 #define COMPAT_PSR_ENDSTATE	COMPAT_PSR_E_BIT

@@ -151,35 +152,9 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->regs[0];
 }
 
-/*
- * Are the current registers suitable for user mode? (used to maintain
- * security in signal handlers)
- */
-static inline int valid_user_regs(struct user_pt_regs *regs)
-{
-	if (user_mode(regs) && (regs->pstate & PSR_I_BIT) == 0) {
-		regs->pstate &= ~(PSR_F_BIT | PSR_A_BIT);
-
-		/* The T bit is reserved for AArch64 */
-		if (!(regs->pstate & PSR_MODE32_BIT))
-			regs->pstate &= ~COMPAT_PSR_T_BIT;
-
-		return 1;
-	}
-
-	/*
-	 * Force PSR to something logical...
-	 */
-	regs->pstate &= PSR_f | PSR_s | (PSR_x & ~PSR_A_BIT) | \
-			COMPAT_PSR_T_BIT | PSR_MODE32_BIT;
-
-	if (!(regs->pstate & PSR_MODE32_BIT)) {
-		regs->pstate &= ~COMPAT_PSR_T_BIT;
-		regs->pstate |= PSR_MODE_EL0t;
-	}
-
-	return 0;
-}
+/* We must avoid circular header include via sched.h */
+struct task_struct;
+int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);
 
 #define instruction_pointer(regs)	((unsigned long)(regs)->pc)
arch/arm64/kernel/ptrace.c +75 -2

92 lines not on this page · raw message

arch/arm64/kernel/signal.c +2 -2
index e18c48c..a8eafdb 100644
@@ -115,7 +115,7 @@ static int restore_sigframe(struct pt_regs *regs,
 	 */
 	regs->syscallno = ~0UL;
 
-	err |= !valid_user_regs(&regs->user_regs);
+	err |= !valid_user_regs(&regs->user_regs, current);
 
 	if (err == 0) {
 		struct fpsimd_context *fpsimd_ctx =

@@ -307,7 +307,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 	/*
 	 * Check that the resulting registers are actually sane.
 	 */
-	ret |= !valid_user_regs(&regs->user_regs);
+	ret |= !valid_user_regs(&regs->user_regs, current);
 
 	/*
 	 * Fast forward the stepping logic so we step into the signal
arch/arm64/kernel/signal32.c +1 -1
index 71ef6dc..1073356 100644
@@ -356,7 +356,7 @@ static int compat_restore_sigframe(struct pt_regs *regs,
 	 */
 	regs->syscallno = ~0UL;
 
-	err |= !valid_user_regs(&regs->user_regs);
+	err |= !valid_user_regs(&regs->user_regs, current);
 
 	aux = (struct compat_aux_sigframe __user *) sf->uc.uc_regspace;
 	if (err == 0)
Signature · 1 line
1.9.1
#1 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

Quoted 29 lines · Mark Rutland

We validate pstate using PSR_MODE32_BIT, which is part of the user-provided pstate (and cannot be trusted). Also, we conflate validation of AArch32 and AArch64 pstate values, making the code difficult to reason about.

Instead, validate the pstate value based on the associated task. The task may or may not be current (e.g. when using ptrace), so this must be passed explicitly by callers. To avoid circular header dependencies via sched.h, is_compat_task is pulled out of asm/ptrace.h.

To make the code possible to reason about, the AArch64 and AArch32 validation is split into separate functions. Software must respect the RES0 policy for SPSR bits, and thus the kernel mirrors the hardware policy (RAZ/WI) for bits as-yet unallocated. When these acquire an architected meaning writes may be permitted (potentially with additional validation).

Signed-off-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Dave Martin <[email protected]>
Cc James Morse <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/ptrace.h | 33 +++---------------
 arch/arm64/kernel/ptrace.c      | 77 +++++++++++++++++++++++++++++++++++++++--
 arch/arm64/kernel/signal.c      |  4 +--
 arch/arm64/kernel/signal32.c    |  2 +-
 4 files changed, 82 insertions(+), 34 deletions(-)

[...]

Quoted 53 lines
arch/arm64/kernel/ptrace.c +29 -2
index ff7f132..5026ffb 100644
@@ -500,7 +500,7 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
 	if (ret)
 		return ret;
 
-	if (!valid_user_regs(&newregs))
+	if (!valid_user_regs(&newregs, target))
 		return -EINVAL;
 
 	task_pt_regs(target)->user_regs = newregs;

@@ -770,7 +770,7 @@ static int compat_gpr_set(struct task_struct *target,
 
 	}
 
-	if (valid_user_regs(&newregs.user_regs))
+	if (valid_user_regs(&newregs.user_regs, target))
 		*task_pt_regs(target) = newregs;
 	else
 		ret = -EINVAL;

@@ -1272,3 +1272,76 @@ asmlinkage void syscall_trace_exit(struct pt_regs *regs)
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
 }
+
+/*
+ * Bits which are always architecturally RES0 per ARM DDI 0487A.h
+ * Userspace cannot use these until they have an architectural meaning.
+ * We also reserve IL for the kernel; SS is handled dynamically.
+ */
+#define SPSR_EL1_AARCH64_RES0_BITS \
+	(GENMASK_ULL(63,32) | GENMASK_ULL(27, 22) | GENMASK_ULL(20, 10) | \
+	 GENMASK_ULL(5, 5))
+#define SPSR_EL1_AARCH32_RES0_BITS \
+	(GENMASK_ULL(63,32) | GENMASK_ULL(24, 22) | GENMASK_ULL(20,20))
+
+static int valid_compat_regs(struct user_pt_regs *regs)
+{
+	regs->pstate &= ~SPSR_EL1_AARCH32_RES0_BITS;
+
+	if (!system_supports_mixed_endian_el0()) {
+		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+			regs->pstate |= COMPAT_PSR_E_BIT;
+		else
+			regs->pstate &= ~COMPAT_PSR_E_BIT;
+	}
+
+	if (user_mode(regs) && (regs->pstate & PSR_MODE32_BIT) &&
+	    (regs->pstate & COMPAT_PSR_A_BIT) == 0 &&
+	    (regs->pstate & COMPAT_PSR_I_BIT) == 0 &&
+	    (regs->pstate & COMPAT_PSR_F_BIT) == 0) {

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1311

Quoted 10 lines
arch/arm64/kernel/ptrace.c +10 -0
+		return 1;
+	}
+
+	/* Force PSR to a valid 32-bit EL0t */
+	regs->pstate &= COMPAT_PSR_N_BIT | COMPAT_PSR_Z_BIT |
+			COMPAT_PSR_C_BIT | COMPAT_PSR_V_BIT |
+			COMPAT_PSR_Q_BIT | COMPAT_PSR_IT_MASK |
+			COMPAT_PSR_GE_MASK | COMPAT_PSR_E_BIT |
+			COMPAT_PSR_T_BIT;
+	regs->pstate |= PSR_MODE32_BIT;

Might be worth an explicit comment to say that we're mirroring arch/arm/ behaviour here.

Quoted 8 lines
arch/arm64/kernel/ptrace.c +8 -0
+
+	return 0;
+}
+
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+	regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 10 lines
arch/arm64/kernel/ptrace.c +10 -0
+	if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+	    (regs->pstate & PSR_D_BIT) == 0 &&
+	    (regs->pstate & PSR_A_BIT) == 0 &&
+	    (regs->pstate & PSR_I_BIT) == 0 &&
+	    (regs->pstate & PSR_F_BIT) == 0) {
+		return 1;
+	}
+
+	/* Force PSR to a valid 64-bit EL0t */
+	regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

Will

#2 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

Mark,

Quoted 24 lines · Will Deacon
Quoted 23 lines · Mark Rutland

We validate pstate using PSR_MODE32_BIT, which is part of the user-provided pstate (and cannot be trusted). Also, we conflate validation of AArch32 and AArch64 pstate values, making the code difficult to reason about.

Instead, validate the pstate value based on the associated task. The task may or may not be current (e.g. when using ptrace), so this must be passed explicitly by callers. To avoid circular header dependencies via sched.h, is_compat_task is pulled out of asm/ptrace.h.

To make the code possible to reason about, the AArch64 and AArch32 validation is split into separate functions. Software must respect the RES0 policy for SPSR bits, and thus the kernel mirrors the hardware policy (RAZ/WI) for bits as-yet unallocated. When these acquire an architected meaning writes may be permitted (potentially with additional validation).

Signed-off-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Dave Martin <[email protected]>
Cc James Morse <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Will Deacon <[email protected]>

[...]

Quoted 41 lines
Quoted 15 lines
arch/arm64/kernel/ptrace.c +15 -0
+static int valid_compat_regs(struct user_pt_regs *regs)
+{
+	regs->pstate &= ~SPSR_EL1_AARCH32_RES0_BITS;
+
+	if (!system_supports_mixed_endian_el0()) {
+		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+			regs->pstate |= COMPAT_PSR_E_BIT;
+		else
+			regs->pstate &= ~COMPAT_PSR_E_BIT;
+	}
+
+	if (user_mode(regs) && (regs->pstate & PSR_MODE32_BIT) &&
+	    (regs->pstate & COMPAT_PSR_A_BIT) == 0 &&
+	    (regs->pstate & COMPAT_PSR_I_BIT) == 0 &&
+	    (regs->pstate & COMPAT_PSR_F_BIT) == 0) {

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1311

Quoted 10 lines
arch/arm64/kernel/ptrace.c +10 -0
+		return 1;
+	}
+
+	/* Force PSR to a valid 32-bit EL0t */
+	regs->pstate &= COMPAT_PSR_N_BIT | COMPAT_PSR_Z_BIT |
+			COMPAT_PSR_C_BIT | COMPAT_PSR_V_BIT |
+			COMPAT_PSR_Q_BIT | COMPAT_PSR_IT_MASK |
+			COMPAT_PSR_GE_MASK | COMPAT_PSR_E_BIT |
+			COMPAT_PSR_T_BIT;
+	regs->pstate |= PSR_MODE32_BIT;

Might be worth an explicit comment to say that we're mirroring arch/arm/ behaviour here.

Quoted 12 lines
arch/arm64/kernel/ptrace.c +12 -0
+
+	return 0;
+}
+
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+	regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+
+	if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+	    (regs->pstate & PSR_D_BIT) == 0 &&
+	    (regs->pstate & PSR_A_BIT) == 0 &&
+	    (regs->pstate & PSR_I_BIT) == 0 &&
Quoted 8 lines

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 6 lines
arch/arm64/kernel/ptrace.c +6 -0
+	    (regs->pstate & PSR_F_BIT) == 0) {
+		return 1;
+	}
+
+	/* Force PSR to a valid 64-bit EL0t */
+	regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

Are you planning to respin this patch?

Will

author reviewer
#3 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

In reply to Will Deacon
Quoted 33 lines · Will Deacon
Quoted 29 lines · Mark Rutland

We validate pstate using PSR_MODE32_BIT, which is part of the user-provided pstate (and cannot be trusted). Also, we conflate validation of AArch32 and AArch64 pstate values, making the code difficult to reason about.

Instead, validate the pstate value based on the associated task. The task may or may not be current (e.g. when using ptrace), so this must be passed explicitly by callers. To avoid circular header dependencies via sched.h, is_compat_task is pulled out of asm/ptrace.h.

To make the code possible to reason about, the AArch64 and AArch32 validation is split into separate functions. Software must respect the RES0 policy for SPSR bits, and thus the kernel mirrors the hardware policy (RAZ/WI) for bits as-yet unallocated. When these acquire an architected meaning writes may be permitted (potentially with additional validation).

Signed-off-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Dave Martin <[email protected]>
Cc James Morse <[email protected]>
Cc Peter Maydell <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/ptrace.h | 33 +++---------------
 arch/arm64/kernel/ptrace.c      | 77 +++++++++++++++++++++++++++++++++++++++--
 arch/arm64/kernel/signal.c      |  4 +--
 arch/arm64/kernel/signal32.c    |  2 +-
 4 files changed, 82 insertions(+), 34 deletions(-)

[...]

Quoted 10 lines

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1311

Quoted 7 lines
arch/arm64/kernel/ptrace.c +7 -0
+	/* Force PSR to a valid 32-bit EL0t */
+	regs->pstate &= COMPAT_PSR_N_BIT | COMPAT_PSR_Z_BIT |
+			COMPAT_PSR_C_BIT | COMPAT_PSR_V_BIT |
+			COMPAT_PSR_Q_BIT | COMPAT_PSR_IT_MASK |
+			COMPAT_PSR_GE_MASK | COMPAT_PSR_E_BIT |
+			COMPAT_PSR_T_BIT;
+	regs->pstate |= PSR_MODE32_BIT;

Might be worth an explicit comment to say that we're mirroring arch/arm/ behaviour here.

Done.

Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/ptrace.c +8 -0
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+	regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+
+	if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+	    (regs->pstate & PSR_D_BIT) == 0 &&
+	    (regs->pstate & PSR_A_BIT) == 0 &&
+	    (regs->pstate & PSR_I_BIT) == 0 &&
Quoted 8 lines

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 6 lines
arch/arm64/kernel/ptrace.c +6 -0
+	    (regs->pstate & PSR_F_BIT) == 0) {
+		return 1;
+	}
+
+	/* Force PSR to a valid 64-bit EL0t */
+	regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

I'm worried that some userspace might be relying on these being preserved. If that turns out to be the case, then adding them back could break userspace that in the meantime ended up relying on these being zeroed.

If we're certain that no-one is relying on these, I can zero them. Otherwise, while it would look neater I'm not sure that we gain much relative to the potential pain we might be causing ourselves.

Thanks,
Mark.

#4 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

Quoted 18 lines · Mark Rutland
Quoted 8 lines · Will Deacon
Quoted 8 lines
arch/arm64/kernel/ptrace.c +8 -0
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+   regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+
+   if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+       (regs->pstate & PSR_D_BIT) == 0 &&
+       (regs->pstate & PSR_A_BIT) == 0 &&
+       (regs->pstate & PSR_I_BIT) == 0 &&
Quoted 8 lines

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 6 lines
arch/arm64/kernel/ptrace.c +6 -0
+       (regs->pstate & PSR_F_BIT) == 0) {
+           return 1;
+   }
+
+   /* Force PSR to a valid 64-bit EL0t */
+   regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

Quoted 2 lines

I'm worried that some userspace might be relying on these being preserved.

This function is called as part of signal-return, right? You clearly can't just zap the flag registers in that code path, because you'd then be corrupting the flags of the bit of userspace code that was interrupted by the signal. (Or am I missing something?)

thanks
-- PMM

#5 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

Quoted 19 lines · Peter Maydell
Quoted 18 lines · Mark Rutland
Quoted 16 lines · Will Deacon
Quoted 4 lines
arch/arm64/kernel/ptrace.c +4 -0
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+   regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 10 lines
arch/arm64/kernel/ptrace.c +10 -0
+   if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+       (regs->pstate & PSR_D_BIT) == 0 &&
+       (regs->pstate & PSR_A_BIT) == 0 &&
+       (regs->pstate & PSR_I_BIT) == 0 &&
+       (regs->pstate & PSR_F_BIT) == 0) {
+           return 1;
+   }
+
+   /* Force PSR to a valid 64-bit EL0t */
+   regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

Quoted 8 lines
Quoted 2 lines

I'm worried that some userspace might be relying on these being preserved.

This function is called as part of signal-return, right? You clearly can't just zap the flag registers in that code path, because you'd then be corrupting the flags of the bit of userspace code that was interrupted by the signal. (Or am I missing something?)

Well, it would only occur if the signal handler had tried to set pstate to an invalid value. That said, it is a change in behaviour, so we can leave it as Mark has suggested.

Will

author reviewer
#6 lore raw

PATCH v3 1/1 arm64: Rework valid_user_regs

Quoted 29 lines · Will Deacon
Quoted 19 lines · Peter Maydell
Quoted 18 lines · Mark Rutland
Quoted 16 lines · Will Deacon
Quoted 4 lines
arch/arm64/kernel/ptrace.c +4 -0
+static int valid_native_regs(struct user_pt_regs *regs)
+{
+   regs->pstate &= ~SPSR_EL1_AARCH64_RES0_BITS;
+

arch/arm64/kernel/ptrace.c asmlinkage void syscall_trace_exit(struct pt_regs *regs) · line 1329

Quoted 10 lines
arch/arm64/kernel/ptrace.c +10 -0
+   if (user_mode(regs) && !(regs->pstate & PSR_MODE32_BIT) &&
+       (regs->pstate & PSR_D_BIT) == 0 &&
+       (regs->pstate & PSR_A_BIT) == 0 &&
+       (regs->pstate & PSR_I_BIT) == 0 &&
+       (regs->pstate & PSR_F_BIT) == 0) {
+           return 1;
+   }
+
+   /* Force PSR to a valid 64-bit EL0t */
+   regs->pstate &= PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;

Can we not just zap the pstate to PSR_MODE_EL0t and be done with it?

Quoted 8 lines
Quoted 2 lines

I'm worried that some userspace might be relying on these being preserved.

This function is called as part of signal-return, right? You clearly can't just zap the flag registers in that code path, because you'd then be corrupting the flags of the bit of userspace code that was interrupted by the signal. (Or am I missing something?)

Quoted 3 lines

Well, it would only occur if the signal handler had tried to set pstate to an invalid value. That said, it is a change in behaviour, so we can leave it as Mark has suggested.

Ok. Assuming you're happy with that I'll post a v4 shortly with the AArch32 comment, and the forced AArch64 state left as in v3.

Thanks,
Mark.