Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964804AbcDYTeK (ORCPT ); Mon, 25 Apr 2016 15:34:10 -0400 Received: from mail-oi0-f46.google.com ([209.85.218.46]:36383 "EHLO mail-oi0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933381AbcDYTeH (ORCPT ); Mon, 25 Apr 2016 15:34:07 -0400 MIME-Version: 1.0 In-Reply-To: <571E5D51.2060809@virtuozzo.com> References: <1461600763-3534-1-git-send-email-dsafonov@virtuozzo.com> <1461600763-3534-3-git-send-email-dsafonov@virtuozzo.com> <571E5076.2040802@virtuozzo.com> <571E5D51.2060809@virtuozzo.com> From: Andy Lutomirski Date: Mon, 25 Apr 2016 12:33:46 -0700 Message-ID: Subject: Re: [RFC 3/3] x86/ptrace: down with test_thread_flag(TIF_IA32) To: Dmitry Safonov Cc: Oleg Nesterov , "linux-kernel@vger.kernel.org" , Dmitry Safonov <0x7f454c46@gmail.com>, Andy Lutomirski , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , X86 ML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3214 Lines: 92 On Mon, Apr 25, 2016 at 11:09 AM, Dmitry Safonov wrote: > On 04/25/2016 08:14 PM, Dmitry Safonov wrote: >> >> On 04/25/2016 07:53 PM, Andy Lutomirski wrote: >>> >>> On Mon, Apr 25, 2016 at 9:12 AM, Dmitry Safonov >>> wrote: >>>> >>>> As the task isn't executing at the moment of {GET,SET}REGS, >>>> return regset that corresponds to code selector. >>>> So, for i386 elf binary that changed it's CS to __USER_CS >>>> it will return full x86_64 register set. >>>> >>>> That will change ABI: i.e, strace uses returned register size >>>> to determine, in which mode the application is. >>>> With the current ABI that way is buggy: >>> >>> Oleg, any comment here? >>> >>>> int main(int argc, char **argv, char **envp) >>>> { >>>> printf("Here we exit\n"); >>>> fflush(stdout); >>>> asm volatile ("int $0x80" : : "a" (__NR_exit), "D" (1)); >>>> printf("After exit\n"); >>>> >>>> return 0; >>>> } >>>> >>>> This program will confuse strace: >>>> >>>> [tst]$ strace ./confuse 2>&1 | tail >>>> brk(0x1ca1000) = 0x1ca1000 >>>> write(1, "Here we exit\n", 13Here we exit >>>> ) = 13 >>>> exit(1) = ? >>>> <... exit resumed> strace: _exit returned! >>>> ) = ? >>>> write(1, "After exit\n", 11After exit >>>> ) = 11 >>>> exit_group(0) = ? >>>> +++ exited with 0 +++ >>>> >>>> So this ABI change should make PTRACE_GETREGSET more reliable and >>>> this will be another step to drop TIF_{IA32,X32} flags. >>> >>> Does strace start working again with this change? I suspect that >>> we'll eventually have to expose syscall_get_arch directly through >>> ptrace, but that's a project for another day. >> >> >> Oh, crap, not yet - seems like, I failed with my test. >> I'll resend this patch as will get it fixed, sorry. > > > I find out, what I have changed (and broke test): >> if (!user_64bit_mode(task_pt_regs(task))) > was >> if (task_thread_info(task)->status & TS_COMPAT) > > That way the test runs now: >> >> brk(NULL) = 0x1145000 >> brk(0x1167000) = 0x1167000 >> write(1, "Here we exit\n", 13Here we exit >> ) = 13 >> strace: [ Process PID=1608 runs in 32 bit mode. ] >> umask(0) = 022 >> strace: [ Process PID=1608 runs in 64 bit mode. ] >> write(1, "After exit\n", 11After exit >> ) = 11 >> exit_group(0) = ? >> +++ exited with 0 +++ > > > But I changed on signal patch rebase and now I'm > thinking: should it be >> if (task_thread_info(task)->status & TS_COMPAT || >> !user_64bit_mode(task_pt_regs(task))) > or what? > Should we count program that does compat syscall > as compatible even if it's in 64-bit mode? I think we should report 64-bit regs if the app is running in 64-bit mode. Then (not necessarily as part of your series), we should have a way for ptrace users to query the *syscall* arch. IOW, this is totally screwed up right now. I think the goal of your patch should be to stop using TIF_IA32 without breaking it any worse than it already is. --Andy