[PATCH 0/2] ARM: honor Thumb bit when calling procinfo setup functions

[PATCH 0/2] ARM: honor Thumb bit when calling procinfo setup functions

[Ard Biesheuvel]

This addresses the observed issue where we are inadvertently relying
on the procinfo setup function to use the same instruction set as the
call site instead of using the correct annotation (patch #1) and call
sequence (patch #2)

Ard Biesheuvel (2):
  ARM: include Thumb2 bit in PROCINFO setup function offset
  ARM: prefer "bx reg" over "mov pc, reg" for all registers

 arch/arm/include/asm/assembler.h | 4 ----
 arch/arm/mm/proc-v7.S            | 2 +-
 arch/arm/mm/proc-v7m.S           | 2 +-
 3 files changed, 2 insertions(+), 6 deletions(-)

-- 
1.8.3.2

[PATCH 2/2] ARM: prefer "bx reg" over "mov pc, reg" for all registers

[Ard Biesheuvel]

Commit 6ebbf2ce437b (ARM: convert all "mov.* pc, reg" to "bx reg"
for ARMv6+) replaced all occurrences of 'mov pc, <reg>' with the
'ret' macro. However, this macro only emits the 'bx' instruction
when used with the 'lr' register, but still uses 'mov pc, <reg>'
for everything else.

Since ARM/Thumb2 interworking is allowed in the static kernel
(i.e., inside vmlinux), this is potentially unsafe, since the mov
instruction will not switch modes based on the Thumb bit.

So instead, emit the 'bx' instruction in all cases, and not just
for the 'lr' register.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/include/asm/assembler.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 186270b3e194..6dda6e3378ea 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -435,11 +435,7 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 #if __LINUX_ARM_ARCH__ < 6
 	mov\c	pc, \reg
 #else
-	.ifeqs	"\reg", "lr"
 	bx\c	\reg
-	.else
-	mov\c	pc, \reg
-	.endif
 #endif
 	.endm
 	.endr
-- 
1.8.3.2

[PATCH 2/2] ARM: prefer "bx reg" over "mov pc, reg" for all registers

[Russell King - ARM Linux]

On Tue, Apr 21, 2015 at 11:09:32AM +0200, Ard Biesheuvel wrote:
> Commit 6ebbf2ce437b (ARM: convert all "mov.* pc, reg" to "bx reg"
> for ARMv6+) replaced all occurrences of 'mov pc, <reg>' with the
> 'ret' macro. However, this macro only emits the 'bx' instruction
> when used with the 'lr' register, but still uses 'mov pc, <reg>'
> for everything else.
> 
> Since ARM/Thumb2 interworking is allowed in the static kernel
> (i.e., inside vmlinux), this is potentially unsafe, since the mov
> instruction will not switch modes based on the Thumb bit.
> 
> So instead, emit the 'bx' instruction in all cases, and not just
> for the 'lr' register.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

NAK.  This is very much intentional, but I'm unable to explain why
publically.  Not every "mov pc, xx" should be a bx instruction.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 0/2] ARM: honor Thumb bit when calling procinfo setup functions

[Russell King - ARM Linux]

On Tue, Apr 21, 2015 at 11:06:43AM +0200, Ard Biesheuvel wrote:
> This addresses the observed issue where we are inadvertently relying
> on the procinfo setup function to use the same instruction set as the
> call site instead of using the correct annotation (patch #1) and call
> sequence (patch #2)
> 
> Ard Biesheuvel (2):
>   ARM: include Thumb2 bit in PROCINFO setup function offset
>   ARM: prefer "bx reg" over "mov pc, reg" for all registers

Patch 1 is missing.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 2/2] ARM: prefer "bx reg" over "mov pc, reg" for all registers

[Ard Biesheuvel]

On 21 April 2015 at 12:07, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Apr 21, 2015 at 11:09:32AM +0200, Ard Biesheuvel wrote:
>> Commit 6ebbf2ce437b (ARM: convert all "mov.* pc, reg" to "bx reg"
>> for ARMv6+) replaced all occurrences of 'mov pc, <reg>' with the
>> 'ret' macro. However, this macro only emits the 'bx' instruction
>> when used with the 'lr' register, but still uses 'mov pc, <reg>'
>> for everything else.
>>
>> Since ARM/Thumb2 interworking is allowed in the static kernel
>> (i.e., inside vmlinux), this is potentially unsafe, since the mov
>> instruction will not switch modes based on the Thumb bit.
>>
>> So instead, emit the 'bx' instruction in all cases, and not just
>> for the 'lr' register.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> NAK.  This is very much intentional, but I'm unable to explain why
> publically.  Not every "mov pc, xx" should be a bx instruction.
>

Hhm, ok. How about only for CONFIG_THUMB2_KERNEL then?

[PATCH 1/2] ARM: include Thumb2 bit in PROCINFO setup function offset

[Ard Biesheuvel]

This updates the PROCINFO offset-to-setup-function fields of the
Thumb2 capable CPU definitions to include the Thumb bit when building
a Thumb2 kernel. This ensures that these function are always called
in the correct mode.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mm/proc-v7.S  | 2 +-
 arch/arm/mm/proc-v7m.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3d1054f11a8a..eb3f66cb8ad0 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -492,7 +492,7 @@ __v7_setup_stack:
 			PMD_SECT_AF | PMD_FLAGS_UP | \mm_mmuflags)
 	.long	PMD_TYPE_SECT | PMD_SECT_AP_WRITE | \
 		PMD_SECT_AP_READ | PMD_SECT_AF | \io_mmuflags
-	initfn	\initfunc, \name
+	initfn	BSYM(\initfunc), \name
 	.long	cpu_arch_name
 	.long	cpu_elf_name
 	.long	HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT | \
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index e08e1f2bab76..e85c3b98d4b6 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -146,7 +146,7 @@ __v7m_proc_info:
 	.long	0x000f0000		@ Mask for ID
 	.long   0			@ proc_info_list.__cpu_mm_mmu_flags
 	.long   0			@ proc_info_list.__cpu_io_mmu_flags
-	initfn	__v7m_setup, __v7m_proc_info	@ proc_info_list.__cpu_flush
+	initfn	BSYM(__v7m_setup), __v7m_proc_info	@ proc_info_list.__cpu_flush
 	.long	cpu_arch_name
 	.long	cpu_elf_name
 	.long	HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT
-- 
1.8.3.2

[PATCH 1/2] ARM: include Thumb2 bit in PROCINFO setup function offset

[Russell King - ARM Linux]

On Tue, Apr 21, 2015 at 12:08:51PM +0200, Ard Biesheuvel wrote:
> This updates the PROCINFO offset-to-setup-function fields of the
> Thumb2 capable CPU definitions to include the Thumb bit when building
> a Thumb2 kernel. This ensures that these function are always called
> in the correct mode.

I don't think this is necessary, in fact, I think this is positively
regression causing.

The symbol 'initfunc' is known to the assembler to be a thumb symbol.
As we have seen already from the kernel dumps from the V7M kernel, when
it calculates initfunc - name in a T2 kernel, the resulting value is an
_odd_ number.

Using BSYM() here will increment it to be the next _even_ number, which
is wrong as this will potentially point at either half way through a
32-bit T2 instruction, or the second 16-bit T2 instruction.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.