linux-kernel - Re: [PATCH] x86/ptrace: Remove questionable TS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b9f316ca-5f79-91f5-1d19-773679778357@palves.net>
Date:	Sat, 18 Jun 2016 14:55:58 +0100
From:	Pedro Alves <pedro@...ves.net>
To:	Andy Lutomirski <luto@...nel.org>, x86@...nel.org
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>,
	Borislav Petkov <bp@...en8.de>, Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH] x86/ptrace: Remove questionable TS_COMPAT usage in ptrace

On 06/18/2016 11:21 AM, Andy Lutomirski wrote:
> A 32-bit tracer can set a tracee's TS_COMPAT flag by poking orig_ax.
> 
> - If the tracee is stopped in a 32-bit syscall, this has no effect
>   as TS_COMPAT will already be set.
> 
> - If the tracee is stopped on entry to a 64-bit syscall, this could
>   cause problems: in_compat_syscall() etc will be out of sync with
>   the actual syscall table in use.  I can imagine this bre aking
>   audit.  (It can't meaningfully break seccomp, as a malicious
>   tracer can already fully bypass seccomp.)  I could also imagine it
>   subtly confusing the implementations of a few syscalls.
> 
>  - If the tracee is stopped in a non-syscall context, then TS_COMPAT
>    will end up set in user mode, which isn't supposed to happen.  The results
>    are likely to be similar to the 64-bit syscall entry case.
> 
> Fix it by deleting the code.
> 
> Here's my understanding of the previous intent.  Suppose a
> 32-bit tracee makes a syscall that returns one of the -ERESTART
> codes.  A 32-bit tracer saves away its register state.  The tracee
> resumes, returns from the system call, and gets stopped again for a
> non-syscall reason (e.g. a signal).  Then the tracer tries to roll
> back the tracee's state by writing all of the saved registers back.
> 
> The result of this sequence of events will be that the tracee's
> registers' low bits match what they were at the end of the syscall
> but TS_COMPAT will be clear.  This will cause syscall_get_error() to
> return a *positive* number, because we zero-extend registers poked
> by 32-bit tracers instead of sign-extending them.  As a result, with
> this change, syscall restart won't happen, whereas, historically, it
> would have happened.
> 
> As far as I can tell, this corner case isn't very important, and I

I believe it's actually very much very important for gdb, for restoring
the inferior state when the user calls a function in the inferior, with:

 (gdb) print foo()

Some background here:

 http://linux-kernel.vger.kernel.narkive.com/fqrh4xKK/patch-x86-ptrace-sign-extend-eax-with-orig-eax-0

This hunk being mentioned in this thread a couple years ago too:

 https://www.sourceware.org/ml/gdb/2014-04/msg00095.html

Please don't break this use case ( and fix the one reported in
that thread :-) ).

Thanks,
Pedro Alves