2016-04-25 22:33 GMT+03:00 Andy Lutomirski <[email protected]>: > On Mon, Apr 25, 2016 at 11:09 AM, Dmitry Safonov <[email protected]> > 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 <[email protected]> >>>> 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.
