[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Leif Lindholm]

The SWP instruction was deprecated in the ARMv6 architecture, superseded
by the LDREX/STREX family of instructions for
load-linked/store-conditional operations. The ARMv7 multiprocessing
extensions mandate that SWP/SWPB instructions are treated as undefined
from reset, with the ability to enable them through the System Control
Register SW bit.

This patch adds the alternative solution to emulate the SWP and SWPB
instructions using LDREX/STREX sequences, and log statistics to
/proc/cpu/swp_emulation. To correctly deal with copy-on-write, it also
modifies cpu_v7_set_pte_ext to change the mappings to priviliged RO when
user RO.

Signed-off-by: Leif Lindholm <leif.lindholm@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/Makefile      |    1 
 arch/arm/kernel/swp_emulate.c |  282 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/Kconfig           |   20 +++
 arch/arm/mm/proc-v7.S         |    6 +
 4 files changed, 309 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/kernel/swp_emulate.c

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index dd00f74..d1befbc 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_ARM_THUMBEE)	+= thumbee.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_ARM_UNWIND)	+= unwind.o
 obj-$(CONFIG_HAVE_TCM)		+= tcm.o
+obj-$(CONFIG_SWP_EMULATE)	+= swp_emulate.o
 
 obj-$(CONFIG_CRUNCH)		+= crunch.o crunch-bits.o
 AFLAGS_crunch-bits.o		:= -Wa,-mcpu=ep9312
diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c
new file mode 100644
index 0000000..51fdbfd
--- /dev/null
+++ b/arch/arm/kernel/swp_emulate.c
@@ -0,0 +1,282 @@
+/*
+ *  linux/arch/arm/kernel/swp_emulate.c
+ *
+ *  Copyright (C) 2009 ARM Limited
+ *  __user_* functions adapted from include/asm/uaccess.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ *  Implements emulation of the SWP/SWPB instructions using load-exclusive and
+ *  store-exclusive for processors that have them disabled (or future ones that
+ *  might not implement them).
+ *
+ *  Syntax of SWP{B} instruction: SWP{B}<c> <Rt>, <Rt2>, [<Rn>]
+ *  Where: Rt  = destination
+ *	   Rt2 = source
+ *	   Rn  = address
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/sched.h>
+#include <linux/syscalls.h>
+
+#include <asm/traps.h>
+#include <asm/uaccess.h>
+
+
+/*
+ * Error-checking SWP macros implemented using ldrex{b}/strex{b}
+ */
+#define __user_swp_asm(data, addr, res)				\
+	__asm__ __volatile__(					\
+	"	mov	r3, %1\n"				\
+	"0:	ldrex	%1, [%2]\n"				\
+	"1:	strex	%0, r3, [%2]\n"				\
+	"	cmp	%0, #0\n"				\
+	"	bne	0b\n"					\
+	"2:\n"							\
+	"	.section .fixup,\"ax\"\n"			\
+	"	.align	2\n"					\
+	"3:	mov	%0, %3\n"				\
+	"	b	2b\n"					\
+	"	.previous\n"					\
+	"	.section __ex_table,\"a\"\n"			\
+	"	.align	3\n"					\
+	"	.long	0b, 3b\n"				\
+	"	.long	1b, 3b\n"				\
+	"	.previous"					\
+	: "=&r" (res), "+r" (data)				\
+	: "r" (addr), "i" (-EFAULT)				\
+	: "cc", "r3")
+
+#define __user_swpb_asm(data, addr, res)			\
+	__asm__ __volatile__(					\
+	"	mov	r3, %1\n"				\
+	"0:	ldrexb	%1, [%2]\n"				\
+	"1:	strexb	%0, r3, [%2]\n"				\
+	"	cmp	%0, #0\n"				\
+	"	bne	0b\n"					\
+	"2:\n"							\
+	"	.section .fixup,\"ax\"\n"			\
+	"	.align	2\n"					\
+	"3:	mov	%0, %3\n"				\
+	"	b	2b\n"					\
+	"	.previous\n"					\
+	"	.section __ex_table,\"a\"\n"			\
+	"	.align	3\n"					\
+	"	.long	0b, 3b\n"				\
+	"	.long	1b, 3b\n"				\
+	"	.previous"					\
+	: "=&r" (res), "+r" (data)				\
+	: "r" (addr), "i" (-EFAULT)				\
+	: "cc", "r3")
+
+/*
+ * Macros/defines for extracting register numbers from instruction.
+ */
+#define EXTRACT_REG_NUM(instruction, offset) \
+  (((instruction) & (0xf << (offset))) >> (offset))
+#define RN_OFFSET  16
+#define RT_OFFSET  12
+#define RT2_OFFSET  0
+
+static unsigned int swpcounter;
+static unsigned int swpbcounter;
+static unsigned int abtcounter;
+static long	    previous_pid;
+
+#ifdef CONFIG_PROC_FS
+static int proc_read_status(char *page, char **start, off_t off, int count,
+			    int *eof, void *data)
+{
+	char *p = page;
+	int len;
+
+	p += sprintf(p, "Emulated SWP:\t\t%u\n", swpcounter);
+	p += sprintf(p, "Emulated SWPB:\t\t%u\n", swpbcounter);
+	p += sprintf(p, "Aborted SWP{B}:\t\t%u\n", abtcounter);
+	if (previous_pid != 0)
+		p += sprintf(p, "Last process:\t\t%ld\n", previous_pid);
+
+	len = (p - page) - off;
+	if (len < 0)
+		len = 0;
+
+	*eof = (len <= count) ? 1 : 0;
+	*start = page + off;
+
+	return len;
+}
+#endif
+
+/*
+ * Set up process info to signal segmentation fault - called on access error.
+ */
+static void set_segfault(struct pt_regs *regs, unsigned long addr)
+{
+	siginfo_t info;
+
+	if (find_vma(current->mm, addr) == NULL)
+		info.si_code  = SEGV_MAPERR;
+	else
+		info.si_code  = SEGV_ACCERR;
+
+	info.si_signo = SIGSEGV;
+	info.si_errno = 0;
+	info.si_addr  = (void __user *)regs->ARM_pc;
+
+	pr_debug("SWP{B} emulation: access caused memory abort!\n");
+	arm_notify_die("Illegal memory access", regs, &info, 0, 0);
+
+	abtcounter++;
+}
+
+static int emulate_swp(struct pt_regs *regs, unsigned int address,
+		       unsigned int destreg, unsigned int data)
+{
+	unsigned int res = 0;
+
+	if (address & 0x3) {
+		/* SWP to unaligned address not permitted */
+		pr_debug("SWP instruction on unaligned pointer!\n");
+		return -EFAULT;
+	}
+
+	smp_mb();
+	__user_swp_asm(data, address, res);
+	smp_mb();
+
+	if (res == -EFAULT)
+		return -EFAULT;
+
+	regs->uregs[destreg] = data;
+	swpcounter++;
+
+	return 0;
+}
+
+static int emulate_swpb(struct pt_regs *regs, unsigned int address,
+			unsigned int destreg, unsigned int data)
+{
+	unsigned int res = 0;
+
+	smp_mb();
+	__user_swpb_asm(data, address, res);
+	smp_mb();
+
+	if (res == -EFAULT)
+		return -EFAULT;
+
+	regs->uregs[destreg] = data;
+	swpbcounter++;
+
+	return 0;
+}
+
+/*
+ * swp_handler emulates SWP/SWPB instructions using the __user* macros defined
+ * above. Logs warning message to console to inform users programs are using
+ * deprecated instructions.
+ */
+static int swp_handler(struct pt_regs *regs, unsigned int instr)
+{
+	unsigned int address, destreg, data;
+	unsigned int res = 0;
+	long current_pid = sys_getpid();
+
+	address = regs->uregs[EXTRACT_REG_NUM(instr, RN_OFFSET)];
+	data	= regs->uregs[EXTRACT_REG_NUM(instr, RT2_OFFSET)];
+	destreg = EXTRACT_REG_NUM(instr, RT_OFFSET);
+
+	if (current_pid != previous_pid) {
+		printk(KERN_WARNING \
+			"\"%s\" (%ld) uses deprecated SWP{B} instruction\n",
+			current->comm, current_pid);
+		previous_pid = current_pid;
+	}
+
+	pr_debug("addr in r%d->0x%08x, dest is r%d, source in r%d->0x%08x)\n",
+		EXTRACT_REG_NUM(instr, RN_OFFSET), address,
+		destreg, EXTRACT_REG_NUM(instr, RT2_OFFSET), data);
+
+	/* Check access in reasonable access range for both SWP and SWPB */
+	if (!access_ok(VERIFY_WRITE, (address & ~3), 4)) {
+		pr_debug("SWP{B} emulation: access to %p not allowed!\n",
+			(void *)address);
+		res = -EFAULT;
+	} else {
+		/*
+		 * Bit 22 of the instruction distinguishes between the SWP and
+		 * SWPB variants (bit set means SWPB).
+		 */
+		if ((instr & (1 << 22)) == 0)
+			res = emulate_swp(regs, address, destreg, data);
+		else
+			res = emulate_swpb(regs, address, destreg, data);
+	}
+
+	/*
+	 * Memory errors do not mean emulation failed.
+	 * Set up signal info to return SEGV, then return OK
+	 */
+	if (res != 0)
+		set_segfault(regs, address);
+
+	/*
+	 * On successful emulation, revert the adjustment to the PC made in
+	 * kernel/traps.c in order to resume@the next instruction instead of
+	 * reexecuting the SWP{B}.
+	 */
+	regs->ARM_pc += 4;
+
+	return 0;
+}
+
+/*
+ * Only emulate SWP/SWPB executed in ARM state/User mode.
+ * The kernel must be SWP free and SWP{B} does not exist in Thumb/ThumbEE.
+ */
+static struct undef_hook swp_hook = {
+	.instr_mask = 0x0fb00ff0,
+	.instr_val  = 0x01000090,
+	.cpsr_mask  = MODE_MASK | PSR_T_BIT | PSR_J_BIT,
+	.cpsr_val   = USR_MODE,
+	.fn	    = swp_handler
+};
+
+/*
+ * Register handler and create status file in /proc/cpu
+ * Invoked as late_initcall, since not needed before init spawned.
+ */
+static int __init swp_emulation_init(void)
+{
+#ifdef CONFIG_PROC_FS
+	struct proc_dir_entry *res;
+
+#ifndef CONFIG_ALIGNMENT_TRAP
+	res = proc_mkdir("cpu", NULL);
+	if (!res)
+		return -ENOMEM;
+
+	res = create_proc_entry("swp_emulation", S_IRUGO, res);
+#else
+	res = create_proc_entry("cpu/swp_emulation", S_IRUGO, NULL);
+#endif
+
+	if (!res)
+		return -ENOMEM;
+
+	res->read_proc = proc_read_status;
+#endif /* CONFIG_PROC_FS */
+
+	printk(KERN_NOTICE "Registering SWP/SWPB emulation handler\n");
+	register_undef_hook(&swp_hook);
+
+	return 0;
+}
+
+late_initcall(swp_emulation_init);
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index dd4698c..1d40bd9 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -630,6 +630,26 @@ config ARM_THUMBEE
 	  Say Y here if you have a CPU with the ThumbEE extension and code to
 	  make use of it. Say N for code that can run on CPUs without ThumbEE.
 
+config SWP_EMULATE
+	bool "Emulate SWP/SWPB instructions"
+	depends on CPU_V7
+	default y if SMP
+	help
+	  ARMv6 architecture deprecates use of the SWP/SWPB instructions.
+	  ARMv7 multiprocessing extensions introduce the ability to disable
+	  these instructions, triggering an undefined instruction exception
+	  when executed. Say Y here to enable software emulation of these
+	  instructions for userspace (not kernel) using LDREX/STREX.
+	  Also creates /proc/cpu/swp_emulation for statistics.
+
+	  NOTE: when accessing uncached shared regions, LDREX/STREX rely
+	  on an external transaction monitoring block called a global
+	  monitor to maintain update atomicity. If your system does not
+	  implement a global monitor, this option is not safe for programs
+	  that are permitted to map uncached memory (CAP_SYS_RAWIO).
+
+	  If unsure, say Y.
+
 config CPU_BIG_ENDIAN
 	bool "Build big-endian kernel"
 	depends on ARCH_SUPPORTS_BIG_ENDIAN
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 3a28521..cb85aeb 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -147,8 +147,10 @@ ENTRY(cpu_v7_set_pte_ext)
 
 	tst	r1, #L_PTE_USER
 	orrne	r3, r3, #PTE_EXT_AP1
+#ifndef CONFIG_SWP_EMULATE
 	tstne	r3, #PTE_EXT_APX
 	bicne	r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
+#endif
 
 	tst	r1, #L_PTE_EXEC
 	orreq	r3, r3, #PTE_EXT_XN
@@ -275,6 +277,10 @@ __v7_setup:
 #ifdef CONFIG_CPU_ENDIAN_BE8
 	orr	r6, r6, #1 << 25		@ big-endian page tables
 #endif
+#ifdef CONFIG_SWP_EMULATE
+	orr     r5, r5, #(1 << 10)              @ set SW bit in "clear"
+	bic     r6, r6, #(1 << 10)              @ clear it in "mmuset"
+#endif
    	mrc	p15, 0, r0, c1, c0, 0		@ read control register
 	bic	r0, r0, r5			@ clear bits them
 	orr	r0, r0, r6			@ set them

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Russell King - ARM Linux]

On Fri, Dec 18, 2009 at 06:04:06PM +0000, Leif Lindholm wrote:
> +static int swp_handler(struct pt_regs *regs, unsigned int instr)
> +{
> +	unsigned int address, destreg, data;
> +	unsigned int res = 0;
> +	long current_pid = sys_getpid();

Kernel functions calling system calls like this is frowned upon.  We
know what the current PID is - it's available from current->pid.  No
function call required.

> +/*
> + * Register handler and create status file in /proc/cpu
> + * Invoked as late_initcall, since not needed before init spawned.
> + */
> +static int __init swp_emulation_init(void)
> +{
> +#ifdef CONFIG_PROC_FS
> +	struct proc_dir_entry *res;
> +
> +#ifndef CONFIG_ALIGNMENT_TRAP
> +	res = proc_mkdir("cpu", NULL);
> +	if (!res)
> +		return -ENOMEM;
> +
> +	res = create_proc_entry("swp_emulation", S_IRUGO, res);
> +#else
> +	res = create_proc_entry("cpu/swp_emulation", S_IRUGO, NULL);
> +#endif

This needs to be cleaned up - we should probably always create the "cpu"
directory so that these don't have to worry about whether it exists or
not.

> +	  NOTE: when accessing uncached shared regions, LDREX/STREX rely
> +	  on an external transaction monitoring block called a global
> +	  monitor to maintain update atomicity. If your system does not
> +	  implement a global monitor, this option is not safe for programs
> +	  that are permitted to map uncached memory (CAP_SYS_RAWIO).

We can trap this case by looking at the L_PTE_MT_* bits in the pte
for the page we're going to be accessing - that's probably a good
idea to ensure that such accesses are trapped, rather than going
head and possibly risking silent corruption.

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Jamie Lokier]

Russell King - ARM Linux wrote:
> On Fri, Dec 18, 2009 at 06:04:06PM +0000, Leif Lindholm wrote:
> > +static int swp_handler(struct pt_regs *regs, unsigned int instr)
> > +{
> > +	unsigned int address, destreg, data;
> > +	unsigned int res = 0;
> > +	long current_pid = sys_getpid();
> 
> Kernel functions calling system calls like this is frowned upon.  We
> know what the current PID is - it's available from current->pid.  No
> function call required.

Quick nitpick in case Leif follows that advice :-)

sys_getpid returns task_tgid_vnr(current).

It is neither ->pid (gettid returns that), nor directly dereferenced
because of the vnr part.  You'll have to decide whether current->tgid
or task_tgid_vnr(current), or current->pid after all are the
appropriate value in this situation.

-- Jamie

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Jamie Lokier]

Russell King - ARM Linux wrote:
> > +	  NOTE: when accessing uncached shared regions, LDREX/STREX rely
> > +	  on an external transaction monitoring block called a global
> > +	  monitor to maintain update atomicity. If your system does not
> > +	  implement a global monitor, this option is not safe for programs
> > +	  that are permitted to map uncached memory (CAP_SYS_RAWIO).
> 
> We can trap this case by looking at the L_PTE_MT_* bits in the pte
> for the page we're going to be accessing - that's probably a good
> idea to ensure that such accesses are trapped, rather than going
> head and possibly risking silent corruption.

After that test, the kernel could even use the SWP/SWPB instructions
if necessary for correct behaviour :-)

Trapping and sending the process a signal would be
a good way to ensure code doing that is fixed.

There might be thread synchronisation code using SWP/SWPB on shared
memory which gets mapped uncached to prevent aliasing.

So would it be more appropriate to trap when the access is to a
non-RAM mapping?  Or are there devices which depend on the SWP bus
cycle on RAM too, for example a PCI device updating RAM by DMA and
using locked cycles?

-- Jamie

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Russell King - ARM Linux]

On Fri, Dec 18, 2009 at 07:48:14PM +0000, Jamie Lokier wrote:
> Russell King - ARM Linux wrote:
> > On Fri, Dec 18, 2009 at 06:04:06PM +0000, Leif Lindholm wrote:
> > > +static int swp_handler(struct pt_regs *regs, unsigned int instr)
> > > +{
> > > +	unsigned int address, destreg, data;
> > > +	unsigned int res = 0;
> > > +	long current_pid = sys_getpid();
> > 
> > Kernel functions calling system calls like this is frowned upon.  We
> > know what the current PID is - it's available from current->pid.  No
> > function call required.
> 
> Quick nitpick in case Leif follows that advice :-)
> 
> sys_getpid returns task_tgid_vnr(current).
> 
> It is neither ->pid (gettid returns that), nor directly dereferenced
> because of the vnr part.  You'll have to decide whether current->tgid
> or task_tgid_vnr(current), or current->pid after all are the
> appropriate value in this situation.

task_tgid_vnr() returns the "thread group" (aka process) ID, and is
what 'ps' spits out for PID.  current->pid is effectively the thread
ID and is unique to every thread.

If you base this decision on the thread group (sys_getpid()), it can
miss threads within a process which may make use of swp - and the
use of swp may not be in the binary but a called library.

If the purpose of this is to find where the swp's are, then you want
to use current->pid, not sys_getpid().

That aside, the printk is a DoS attack waiting to happen.  It needs
rate limiting.

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Russell King - ARM Linux]

On Fri, Dec 18, 2009 at 07:54:32PM +0000, Jamie Lokier wrote:
> There might be thread synchronisation code using SWP/SWPB on shared
> memory which gets mapped uncached to prevent aliasing.

The shared mmap fixup doesn't get triggered on ARMv7 (or any VIPT
ARM cached system).  Such fixup is only applicable to VIVT ARMs.

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Catalin Marinas]

On Fri, 2009-12-18 at 18:20 +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 18, 2009 at 06:04:06PM +0000, Leif Lindholm wrote:
> > +static int swp_handler(struct pt_regs *regs, unsigned int instr)
> > +{
> > +     unsigned int address, destreg, data;
> > +     unsigned int res = 0;
> > +     long current_pid = sys_getpid();
> 
> Kernel functions calling system calls like this is frowned upon.  We
> know what the current PID is - it's available from current->pid.  No
> function call required.

Actually, current->pid together with get_task_comm() would be even more
useful since the task may die by the time you check the running apps.

> > +       NOTE: when accessing uncached shared regions, LDREX/STREX rely
> > +       on an external transaction monitoring block called a global
> > +       monitor to maintain update atomicity. If your system does not
> > +       implement a global monitor, this option is not safe for programs
> > +       that are permitted to map uncached memory (CAP_SYS_RAWIO).
> 
> We can trap this case by looking at the L_PTE_MT_* bits in the pte
> for the page we're going to be accessing - that's probably a good
> idea to ensure that such accesses are trapped, rather than going
> head and possibly risking silent corruption.

I wonder if there could be an exploit with Leif's current
implementation. The LDREX/STREX pair to uncached or device memory may
never complete. This way a user SWP to something like the frame buffer
would lock the kernel.

There's also TI's issue with SWP to some memory shared with devices
outside the CPU coherency domain where SWP may still be needed. Is this
usage in user space or kernel device drivers?

The major issue I see with SWP is that it isn't guaranteed to always
work in an SMP shareability domain (bus locking not implemented), hence
the LDREX/STREX emulation.

But Leif is away for the rest of the year, so we won't see an updated
patch before January.

-- 
Catalin

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Russell King - ARM Linux]

On Sat, Dec 19, 2009 at 05:18:42PM +0000, Catalin Marinas wrote:
> On Fri, 2009-12-18 at 18:20 +0000, Russell King - ARM Linux wrote:
> > On Fri, Dec 18, 2009 at 06:04:06PM +0000, Leif Lindholm wrote:
> > > +static int swp_handler(struct pt_regs *regs, unsigned int instr)
> > > +{
> > > +     unsigned int address, destreg, data;
> > > +     unsigned int res = 0;
> > > +     long current_pid = sys_getpid();
> > 
> > Kernel functions calling system calls like this is frowned upon.  We
> > know what the current PID is - it's available from current->pid.  No
> > function call required.
> 
> Actually, current->pid together with get_task_comm() would be even more
> useful since the task may die by the time you check the running apps.

That's effectively what is being done - current->comm is effectively
get_task_comm() - get_task_comm() just copies the string out under a
lock.

For debugging of the 'current' task, the additional copy and lock are
mere overhead.

> > We can trap this case by looking at the L_PTE_MT_* bits in the pte
> > for the page we're going to be accessing - that's probably a good
> > idea to ensure that such accesses are trapped, rather than going
> > head and possibly risking silent corruption.
> 
> I wonder if there could be an exploit with Leif's current
> implementation. The LDREX/STREX pair to uncached or device memory may
> never complete. This way a user SWP to something like the frame buffer
> would lock the kernel.

That means we shouldn't consider applying it until that has been
investigated.

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Leif Lindholm]

> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: 19 December 2009 17:29

> > I wonder if there could be an exploit with Leif's current
> > implementation. The LDREX/STREX pair to uncached or device memory may
> > never complete. This way a user SWP to something like the frame
> > buffer would lock the kernel.
>
> That means we shouldn't consider applying it until that has been
> investigated.

What if I modify the patch such that a failed STREX causes the emulation to
return success without readjusting the PC? This would result in the SWP
instruction being executed again upon returning to the application - removing
the potential kernel lockup.

/
        Leif

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Jamie Lokier]

Leif Lindholm wrote:
> > From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> > Sent: 19 December 2009 17:29
> 
> > > I wonder if there could be an exploit with Leif's current
> > > implementation. The LDREX/STREX pair to uncached or device memory may
> > > never complete. This way a user SWP to something like the frame
> > > buffer would lock the kernel.
> >
> > That means we shouldn't consider applying it until that has been
> > investigated.
> 
> What if I modify the patch such that a failed STREX causes the emulation to
> return success without readjusting the PC? This would result in the SWP
> instruction being executed again upon returning to the application - removing
> the potential kernel lockup.

There is no need to return to userspace, as you know that userspace
will immediately retry the instruction, or respond to a signal.

So you can loop, as long as you:

   1. Allow other tasks to run by calling cond_resched().
   2. Break out of the loop if signal_pending().

I think that's sufficient but I hope someone will double check because
I'm a little rusty.

If someone executes SWP on uncached or device memory and it's not an
intentional DOS (e.g. worked with older ARMs), they won't expect the
instruction to loop at that point.  Faulting would be better.
Is it feasible to detect when this has happened and send a SIGBUS
signal instead of looping?

-- Jamie

[PATCH] ARM: Add SWP/SWPB emulation for ARMv7 processors (v2)

[Leif Lindholm]

> From: Jamie Lokier [mailto:jamie at shareable.org]
> Sent: 04 January 2010 19:34

> There is no need to return to userspace, as you know that userspace
> will immediately retry the instruction, or respond to a signal.
> 
> So you can loop, as long as you:
> 
>    1. Allow other tasks to run by calling cond_resched().
>    2. Break out of the loop if signal_pending().

Thanks for these suggestions - I will be posting an updated patch that does
this shortly.
 
> If someone executes SWP on uncached or device memory and it's not an
> intentional DOS (e.g. worked with older ARMs), they won't expect the
> instruction to loop at that point.

True, but difficult to solve in a clean way (see below).

> Faulting would be better. Is it feasible to detect when this has
> happened and send a SIGBUS signal instead of looping?

Only if the processor implements it (the AXI spec explicitly permits a
processor to treat an OKAY response, as opposed to EXOKAY, as an error -
but it doesn't mandate it). I am not aware of any ARM processors that do
this. Otherwise that would certainly be the best solution.

Of course this means that if we want to "detect" an impossible situation
in order to generate an abort, we need to resort to counters, and
determining an arbitrary "highest possible" number of valid sequential
failures...

/
    Leif