-------- Original Message -------- Subject: [PATCH] ARM: Fix restoration of IP scratch register when auditing syscalls Date: Sun, 29 Apr 2012 02:38:24 -0400 From: Jon Masters jcm@jonmasters.org To: Linux ARM Kernel linux-arm-kernel@lists.infradead.org CC: Jon Masters jcm@redhat.com, Al Viro viro@zeniv.linux.org.uk, Eric Paris eparis@redhat.com, Jon Masters jcm@jonmasters.org
The audit subsystem builds upon ptrace to record system calls. This is done in a couple of places (on return from fork into a new task, on exit from the SWI vector), using calls to syscall_trace. The latter function abuses the userspace intra-procedure scratch register (regs->ARM_ip, aka r12), and intends to restore it prior to return to userspace. Unfortunately, there are cases where we will return to userspace without restoring.
If we are in fact not ptracing but are merely auditing calls, we will happily trash the content of ip but will exit to userspace without restoring the value. It just so happens that GLIBC uses ip as a storage for the TLS thread pointer info, and bad things result.
The fix is simply to have an additional out when not ptracing.
Signed-off-by: Jon Masters jcm@jonmasters.org --- arch/arm/kernel/ptrace.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c index ede6443..7b05b6e 100644 --- a/arch/arm/kernel/ptrace.c +++ b/arch/arm/kernel/ptrace.c @@ -928,10 +928,10 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno) audit_syscall_entry(AUDIT_ARCH_NR, scno, regs->ARM_r0, regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
- if (!test_thread_flag(TIF_SYSCALL_TRACE)) - return scno; + if (!test_thread_flag(TIF_SYSCALL_TRACE)) + goto out_no_ptrace; if (!(current->ptrace & PT_PTRACED)) - return scno; + goto out_no_ptrace;
current_thread_info()->syscall = scno;
@@ -951,4 +951,8 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno) regs->ARM_ip = ip;
return current_thread_info()->syscall; + +out_no_ptrace: + regs->ARM_ip = ip; + return scno; }
On Sun, Apr 29, 2012 at 02:40:01AM -0400, Jon Masters wrote:
-------- Original Message -------- Subject: [PATCH] ARM: Fix restoration of IP scratch register when auditing syscalls Date: Sun, 29 Apr 2012 02:38:24 -0400 From: Jon Masters jcm@jonmasters.org To: Linux ARM Kernel linux-arm-kernel@lists.infradead.org CC: Jon Masters jcm@redhat.com, Al Viro viro@zeniv.linux.org.uk, Eric Paris eparis@redhat.com, Jon Masters jcm@jonmasters.org
The audit subsystem builds upon ptrace to record system calls. This is done in a couple of places (on return from fork into a new task, on exit from the SWI vector), using calls to syscall_trace. The latter function abuses the userspace intra-procedure scratch register (regs->ARM_ip, aka r12), and intends to restore it prior to return to userspace. Unfortunately, there are cases where we will return to userspace without restoring.
If we are in fact not ptracing but are merely auditing calls, we will happily trash the content of ip but will exit to userspace without restoring the value. It just so happens that GLIBC uses ip as a storage for the TLS thread pointer info, and bad things result.
The fix is simply to have an additional out when not ptracing.
Signed-off-by: Jon Masters jcm@jonmasters.org
Going to assume you forwarded this here because you want it applied to the Fedora kernel. Likely F17/rawhide?
I'm guessing we probably want to follow http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/096951.html for a bit to see if there are any more iterations of this?
Since it's CC'd to stable, it should get picked up rather quickly if RMK applies it.
josh
On Mon, 2012-04-30 at 12:47 -0400, Josh Boyer wrote:
On Sun, Apr 29, 2012 at 02:40:01AM -0400, Jon Masters wrote:
Going to assume you forwarded this here because you want it applied to the Fedora kernel. Likely F17/rawhide?
I'm guessing we probably want to follow http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/096951.html for a bit to see if there are any more iterations of this?
Since it's CC'd to stable, it should get picked up rather quickly if RMK applies it.
You probably want to follow this thread as it's where an ARM developer actually looked at it :)
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/097002.html
-Eric
On Mon, Apr 30, 2012 at 12:50:56PM -0400, Eric Paris wrote:
On Mon, 2012-04-30 at 12:47 -0400, Josh Boyer wrote:
On Sun, Apr 29, 2012 at 02:40:01AM -0400, Jon Masters wrote:
Going to assume you forwarded this here because you want it applied to the Fedora kernel. Likely F17/rawhide?
I'm guessing we probably want to follow http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/096951.html for a bit to see if there are any more iterations of this?
Since it's CC'd to stable, it should get picked up rather quickly if RMK applies it.
You probably want to follow this thread as it's where an ARM developer actually looked at it :)
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/097002.html
Yep, saw that too. Guess I'll keep my eyes peeled.
josh
On 04/30/2012 12:56 PM, Josh Boyer wrote:
On Mon, Apr 30, 2012 at 12:50:56PM -0400, Eric Paris wrote:
On Mon, 2012-04-30 at 12:47 -0400, Josh Boyer wrote:
On Sun, Apr 29, 2012 at 02:40:01AM -0400, Jon Masters wrote:
Going to assume you forwarded this here because you want it applied to the Fedora kernel. Likely F17/rawhide?
I'm guessing we probably want to follow http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/096951.html for a bit to see if there are any more iterations of this?
Since it's CC'd to stable, it should get picked up rather quickly if RMK applies it.
You probably want to follow this thread as it's where an ARM developer actually looked at it :)
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/097002.html
Yep, saw that too. Guess I'll keep my eyes peeled.
Yea, as Russell says, the original code in there for audit was "complete crap". My fix was just to get rid of obvious register corruption after wasting a lot of time hunting it down, but I did not extensively test audit to see that it was actually working since it seemed obvious there would be other problems, too (you know, given this, who knows what else is wrong - clearly nobody uses it). Will's version is probably where we'll go for this issue. I'm going to test his patch later and then see if audit actually works properly with some further tests - again, leaning toward turning off audit on ARM anyway until we know there are no more horrors, given this little shocker.
Jon.
P.S. Technically OOTO today. I'll test the latest patch tonight and followup - yes, I did forward assuming you'd want to pull, and next time I'll try harder to spell out my intent/request, etc.
kernel@lists.fedoraproject.org
-
Eric Paris -
Jon Masters -
Josh Boyer