From a077224fd35b2f7fbc93f14cf67074fc792fbac2 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 3 Sep 2015 13:24:40 +0100 Subject: [PATCH 1/8] ARM: 8429/1: disable GCC SRA optimization While working on the 32-bit ARM port of UEFI, I noticed a strange corruption in the kernel log. The following snprintf() statement (in drivers/firmware/efi/efi.c:efi_md_typeattr_format()) snprintf(pos, size, "|%3s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]", was producing the following output in the log: | | | | | |WB|WT|WC|UC] | | | | | |WB|WT|WC|UC] | | | | | |WB|WT|WC|UC] |RUN| | | | |WB|WT|WC|UC]* |RUN| | | | |WB|WT|WC|UC]* | | | | | |WB|WT|WC|UC] |RUN| | | | |WB|WT|WC|UC]* | | | | | |WB|WT|WC|UC] |RUN| | | | | | | |UC] |RUN| | | | | | | |UC] As it turns out, this is caused by incorrect code being emitted for the string() function in lib/vsprintf.c. The following code if (!(spec.flags & LEFT)) { while (len < spec.field_width--) { if (buf < end) *buf = ' '; ++buf; } } for (i = 0; i < len; ++i) { if (buf < end) *buf = *s; ++buf; ++s; } while (len < spec.field_width--) { if (buf < end) *buf = ' '; ++buf; } when called with len == 0, triggers an issue in the GCC SRA optimization pass (Scalar Replacement of Aggregates), which handles promotion of signed struct members incorrectly. This is a known but as yet unresolved issue. (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65932). In this particular case, it is causing the second while loop to be executed erroneously a single time, causing the additional space characters to be printed. So disable the optimization by passing -fno-ipa-sra. Cc: Acked-by: Nicolas Pitre Signed-off-by: Ard Biesheuvel Signed-off-by: Russell King --- arch/arm/Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 7451b447cc2d..2c2b28ee4811 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -54,6 +54,14 @@ AS += -EL LD += -EL endif +# +# The Scalar Replacement of Aggregates (SRA) optimization pass in GCC 4.9 and +# later may result in code being generated that handles signed short and signed +# char struct members incorrectly. So disable it. +# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65932) +# +KBUILD_CFLAGS += $(call cc-option,-fno-ipa-sra) + # This selects which instruction set is used. # Note that GCC does not numerically define an architecture version # macro, but instead defines a whole series of macros which makes From 39dc53deff30d9b239ac36cfeb0ef2022d03a449 Mon Sep 17 00:00:00 2001 From: Russell King Date: Mon, 7 Sep 2015 00:29:15 +0100 Subject: [PATCH 2/8] ARM: swpan: fix nwfpe for uaccess changes NWFPE needs to access userspace to check whether the next instruction is another FP instruction. Allow userspace access for this read. Signed-off-by: Russell King --- arch/arm/nwfpe/entry.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm/nwfpe/entry.S b/arch/arm/nwfpe/entry.S index 71df43547659..39c20afad7ed 100644 --- a/arch/arm/nwfpe/entry.S +++ b/arch/arm/nwfpe/entry.S @@ -95,9 +95,10 @@ emulate: reteq r4 @ no, return failure next: + uaccess_enable r3 .Lx1: ldrt r6, [r5], #4 @ get the next instruction and @ increment PC - + uaccess_disable r3 and r2, r6, #0x0F000000 @ test for FP insns teq r2, #0x0C000000 teqne r2, #0x0D000000 From 296254f3223d201f2aa53f5f717eedfdc63f3db8 Mon Sep 17 00:00:00 2001 From: Russell King Date: Mon, 7 Sep 2015 00:30:06 +0100 Subject: [PATCH 3/8] ARM: uaccess: remove unneeded uaccess_save_and_disable macro This macro is never referenced, remove it. Signed-off-by: Russell King --- arch/arm/include/asm/assembler.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h index 3ae0eda5e64f..9007c518d1d8 100644 --- a/arch/arm/include/asm/assembler.h +++ b/arch/arm/include/asm/assembler.h @@ -487,11 +487,6 @@ THUMB( orr \reg , \reg , #PSR_T_BIT ) #endif .endm - .macro uaccess_save_and_disable, tmp - uaccess_save \tmp - uaccess_disable \tmp - .endm - .irp c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo .macro ret\c, reg #if __LINUX_ARM_ARCH__ < 6 From af4cb25df93d2e7a97d65db2bfacaa4400988dea Mon Sep 17 00:00:00 2001 From: Russell King Date: Wed, 9 Sep 2015 21:19:49 +0100 Subject: [PATCH 4/8] ARM: uaccess: fix undefined instruction on ARMv7M/noMMU The use of get_domain() in copy_thread() results in an oops on ARMv7M/noMMU systems. The thread cpu_domain value is only used when CONFIG_CPU_USE_DOMAINS is enabled, so there's no need to save the value in copy_thread() except when this is enabled, and this option will never be enabled on these platforms. Unhandled exception: IPSR = 00000006 LR = fffffff1 CPU: 0 PID: 0 Comm: swapper Not tainted 4.2.0-next-20150909-00001-gb8ec5ad #41 Hardware name: NXP LPC18xx/43xx (Device Tree) task: 2823fbe0 ti: 2823c000 task.ti: 2823c000 PC is at copy_thread+0x18/0x92 LR is at copy_thread+0x19/0x92 pc : [<2800a46e>] lr : [<2800a46f>] psr: 4100000b sp : 2823df00 ip : 00000000 fp : 287c81c0 r10: 00000000 r9 : 00800300 r8 : 287c8000 r7 : 287c8000 r6 : 2818908d r5 : 00000000 r4 : 287ca000 r3 : 00000000 r2 : 00000000 r1 : fffffff0 r0 : 287ca048 xPSR: 4100000b Reported-by: Ariel D'Alessandro Signed-off-by: Russell King --- arch/arm/kernel/process.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 3f18098dfd08..e550a4541f48 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -233,6 +233,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, memset(&thread->cpu_context, 0, sizeof(struct cpu_context_save)); +#ifdef CONFIG_CPU_USE_DOMAINS /* * Copy the initial value of the domain access control register * from the current thread: thread->addr_limit will have been @@ -240,6 +241,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, * kernel/fork.c */ thread->cpu_domain = get_domain(); +#endif if (likely(!(p->flags & PF_KTHREAD))) { *childregs = *current_pt_regs(); From 716ff1921a86c637b8875c7bb312fc6755fa9300 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 11 Sep 2015 08:17:39 +0100 Subject: [PATCH 5/8] ARM: domains: thread_info.h no longer needs asm/domains.h As of 1eef5d2f1b46 ("ARM: domains: switch to keeping domain value in register") we no longer need to include asm/domains.h into asm/thread_info.h. Remove it. Tested-by: Robert Jarzmik Signed-off-by: Russell King --- arch/arm/include/asm/thread_info.h | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h index 0a0aec410d8c..ae02e68b61fc 100644 --- a/arch/arm/include/asm/thread_info.h +++ b/arch/arm/include/asm/thread_info.h @@ -25,7 +25,6 @@ struct task_struct; #include -#include typedef unsigned long mm_segment_t; From 6e8f580d1fcc18e290713984c379cb97131c015a Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 11 Sep 2015 08:34:52 +0100 Subject: [PATCH 6/8] ARM: domains: add memory dependencies to get_domain/set_domain We need to have memory dependencies on get_domain/set_domain to avoid the compiler over-optimising these inline assembly instructions. Loads/stores must not be reordered across a set_domain(), so introduce a compiler barrier for that assembly. The value of get_domain() must not be cached across a set_domain(), but we still want to allow the compiler to optimise it away. Introduce a dependency on current_thread_info()->cpu_domain to avoid this; the new memory clobber in set_domain() should therefore cause the compiler to re-load this. The other advantage of using this is we should have its address in the register set already, or very soon after at most call sites. Tested-by: Robert Jarzmik Signed-off-by: Russell King --- arch/arm/include/asm/domain.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h index e878129f2fee..fc8ba1663601 100644 --- a/arch/arm/include/asm/domain.h +++ b/arch/arm/include/asm/domain.h @@ -12,6 +12,7 @@ #ifndef __ASSEMBLY__ #include +#include #endif /* @@ -89,7 +90,8 @@ static inline unsigned int get_domain(void) asm( "mrc p15, 0, %0, c3, c0 @ get domain" - : "=r" (domain)); + : "=r" (domain) + : "m" (current_thread_info()->cpu_domain)); return domain; } @@ -98,7 +100,7 @@ static inline void set_domain(unsigned val) { asm volatile( "mcr p15, 0, %0, c3, c0 @ set domain" - : : "r" (val)); + : : "r" (val) : "memory"); isb(); } From 0b61f2c0f37983c98ed4207f3f5e265938371b68 Mon Sep 17 00:00:00 2001 From: Julien Grall Date: Fri, 11 Sep 2015 17:25:59 +0100 Subject: [PATCH 7/8] arm/xen: Enable user access to the kernel before issuing a privcmd call When Xen is copying data to/from the guest it will check if the kernel has the right to do the access. If not, the hypercall will return an error. After the commit a5e090acbf545c0a3b04080f8a488b17ec41fe02 "ARM: software-based privileged-no-access support", the kernel can't access any longer the user space by default. This will result to fail on every hypercall made by the userspace (i.e via privcmd). We have to enable the userspace access and then restore the correct permission every time the privcmd is used to made an hypercall. I didn't find generic helpers to do a these operations, so the change is only arm32 specific. Reported-by: Riku Voipio Signed-off-by: Julien Grall Signed-off-by: Russell King --- arch/arm/xen/hypercall.S | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S index f00e08075938..10fd99c568c6 100644 --- a/arch/arm/xen/hypercall.S +++ b/arch/arm/xen/hypercall.S @@ -98,8 +98,23 @@ ENTRY(privcmd_call) mov r1, r2 mov r2, r3 ldr r3, [sp, #8] + /* + * Privcmd calls are issued by the userspace. We need to allow the + * kernel to access the userspace memory before issuing the hypercall. + */ + uaccess_enable r4 + + /* r4 is loaded now as we use it as scratch register before */ ldr r4, [sp, #4] __HVC(XEN_IMM) + + /* + * Disable userspace access from kernel. This is fine to do it + * unconditionally as no set_fs(KERNEL_DS)/set_fs(get_ds()) is + * called before. + */ + uaccess_disable r4 + ldm sp!, {r4} ret lr ENDPROC(privcmd_call); From a4a5a7379e4ca03c192b732d61e446994eb67bbc Mon Sep 17 00:00:00 2001 From: Robert Jarzmik Date: Fri, 11 Sep 2015 17:12:27 +0100 Subject: [PATCH 8/8] ARM: 8431/1: fix alignement of __bug_table section entries On old ARM chips, unaligned accesses to memory are not trapped and fixed. On module load, symbols are relocated, and the relocation of __bug_table symbols is done on a u32 basis. Yet the section is not aligned to a multiple of 4 address, but to a multiple of 2. This triggers an Oops on pxa architecture, where address 0xbf0021ea is the first relocation in the __bug_table section : apply_relocate(): pxa3xx_nand: section 13 reloc 0 sym '' Unable to handle kernel paging request at virtual address bf0021ea pgd = e1cd0000 [bf0021ea] *pgd=c1cce851, *pte=c1cde04f, *ppte=c1cde01f Internal error: Oops: 23 [#1] ARM Modules linked in: CPU: 0 PID: 606 Comm: insmod Not tainted 4.2.0-rc8-next-20150828-cm-x300+ #887 Hardware name: CM-X300 module task: e1c68700 ti: e1c3e000 task.ti: e1c3e000 PC is at apply_relocate+0x2f4/0x3d4 LR is at 0xbf0021ea pc : [] lr : [] psr: 80000013 sp : e1c3fe30 ip : 60000013 fp : e49e8c60 r10: e49e8fa8 r9 : 00000000 r8 : e49e7c58 r7 : e49e8c38 r6 : e49e8a58 r5 : e49e8920 r4 : e49e8918 r3 : bf0021ea r2 : bf007034 r1 : 00000000 r0 : bf000000 Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 0000397f Table: c1cd0018 DAC: 00000051 Process insmod (pid: 606, stack limit = 0xe1c3e198) [] (apply_relocate) from [] (load_module+0x1248/0x1f5c) [] (load_module) from [] (SyS_init_module+0xe4/0x170) [] (SyS_init_module) from [] (ret_fast_syscall+0x0/0x38) Fix this by ensuring entries in __bug_table are all aligned to at least of multiple of 4. This transforms a module section __bug_table as : - [12] __bug_table PROGBITS 00000000 002232 000018 00 A 0 0 1 + [12] __bug_table PROGBITS 00000000 002232 000018 00 A 0 0 4 Signed-off-by: Robert Jarzmik Reviewed-by: Dave Martin Signed-off-by: Russell King --- arch/arm/include/asm/bug.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h index b274bde24905..e7335a92144e 100644 --- a/arch/arm/include/asm/bug.h +++ b/arch/arm/include/asm/bug.h @@ -40,6 +40,7 @@ do { \ "2:\t.asciz " #__file "\n" \ ".popsection\n" \ ".pushsection __bug_table,\"a\"\n" \ + ".align 2\n" \ "3:\t.word 1b, 2b\n" \ "\t.hword " #__line ", 0\n" \ ".popsection"); \