Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964931AbcDYUh1 (ORCPT ); Mon, 25 Apr 2016 16:37:27 -0400 Received: from mail-qk0-f182.google.com ([209.85.220.182]:36240 "EHLO mail-qk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933455AbcDYUhX (ORCPT ); Mon, 25 Apr 2016 16:37:23 -0400 MIME-Version: 1.0 In-Reply-To: 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: Dmitry Safonov <0x7f454c46@gmail.com> Date: Mon, 25 Apr 2016 23:37:03 +0300 Message-ID: Subject: Re: [RFC 3/3] x86/ptrace: down with test_thread_flag(TIF_IA32) To: Andy Lutomirski Cc: Dmitry Safonov , Oleg Nesterov , "linux-kernel@vger.kernel.org" , 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: 3507 Lines: 99 2016-04-25 22:33 GMT+03:00 Andy Lutomirski : > 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. Ok, so that's what that patch does. Big thanks, Andy -- I appreciate your help so much with this patches. -- Regards, Safonov Dmitry.