From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751466AbcFRN4K (ORCPT ); Sat, 18 Jun 2016 09:56:10 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:33339 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751228AbcFRN4I (ORCPT ); Sat, 18 Jun 2016 09:56:08 -0400 Subject: Re: [PATCH] x86/ptrace: Remove questionable TS_COMPAT usage in ptrace To: Andy Lutomirski , x86@kernel.org References: <94bda8cd5f326ae5591c80fb5d7c1c22624accec.1466244711.git.luto@kernel.org> Cc: "linux-kernel@vger.kernel.org" , Kees Cook , Borislav Petkov , Oleg Nesterov From: Pedro Alves Message-ID: Date: Sat, 18 Jun 2016 14:55:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <94bda8cd5f326ae5591c80fb5d7c1c22624accec.1466244711.git.luto@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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