From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D9B0C04AAA for ; Fri, 3 May 2019 16:21:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E859020651 for ; Fri, 3 May 2019 16:21:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=amacapital-net.20150623.gappssmtp.com header.i=@amacapital-net.20150623.gappssmtp.com header.b="AKvvYHpJ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728204AbfECQVA (ORCPT ); Fri, 3 May 2019 12:21:00 -0400 Received: from mail-pg1-f193.google.com ([209.85.215.193]:38519 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727022AbfECQU7 (ORCPT ); Fri, 3 May 2019 12:20:59 -0400 Received: by mail-pg1-f193.google.com with SMTP id j26so2955544pgl.5 for ; Fri, 03 May 2019 09:20:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amacapital-net.20150623.gappssmtp.com; s=20150623; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=KZ7M2mWr1HgOjFYLO00FIxnzVOAK+l4FuiGi1wr1wow=; b=AKvvYHpJ2edjcm0204j+PW95iowk2lDK7GCc0BA/nSaF6MDeF0AIRnHVXuk6WBOY24 FcaQymKNmQJ6pm2GWVojQ23VP1xtGzSP22dM3GLay0Sv1n+qpV809/ndAM30cjG3Qtcj S+z06FlWFGt+qUrsK7BrZD/QgWeRQQPOQl0JH5GrW7V0zKM1bmFzlf6OsH/w7K3roSun hJ7ZNZsvmswPBpD+I1pVnI1LmG33ZWGhrCx5WkmMteKy0ij31Mg+QopA8cGZ9m9GpRgV qCNybIBlnqbR8jGguO7m5y/C1qwwPhDgrs0lwmsWsMdgGLFnzB643lNtdv/V/V55e+r4 JgCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=KZ7M2mWr1HgOjFYLO00FIxnzVOAK+l4FuiGi1wr1wow=; b=IMaLqZl1ad+51tnxCY5ZdF9W3QU22C5Sd9KyWoDWMhxyJbVutA0W0HHqqqCnBrGWWb K4pJnsZ7Nz/j20j6XK+vmt0NHhzWmVS+GeiWT9GPnJJU4BbL2wk2h7LhtsKT7iArtVyc f+yizFjchgZhqjmWAsKHtiSqRlf7xTv09yg6UDFphElMze4IvIOhCvO5YIdyFTwHKwMb L3c5LIwts7UUI9OZhSRGDg0BApL54USbEY27ReCSgG8pun4c+FNqOcBhummP35qr6ovf cFHlKuViekvUQMLGnfIREh/3C+kRJqGt2NhWPgn25nPwWHhpdYzts0q1Dx2h+TtbN9AG 3d9Q== X-Gm-Message-State: APjAAAVoTajHWcTr9MgLmGTAjWzOIS3axAIvAEy/joyd7Ry17xmGp92n jLMwJJfyHM8Quod065awjAOTwQ== X-Google-Smtp-Source: APXvYqz27PSp/BKOF8HBP5MdA5oDqaxXNQsgs6iiXXWd28RIrWLPk7vwjcnah74eFGU3dKeS0GbWjw== X-Received: by 2002:a63:f503:: with SMTP id w3mr11524403pgh.60.1556900457569; Fri, 03 May 2019 09:20:57 -0700 (PDT) Received: from ?IPv6:2600:1010:b025:b35:85ec:5278:6fc6:7ef4? ([2600:1010:b025:b35:85ec:5278:6fc6:7ef4]) by smtp.gmail.com with ESMTPSA id x4sm3519353pfm.19.2019.05.03.09.20.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 May 2019 09:20:56 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (1.0) Subject: Re: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions From: Andy Lutomirski X-Mailer: iPhone Mail (16E227) In-Reply-To: <20190503092247.20cc1ff0@gandalf.local.home> Date: Fri, 3 May 2019 09:20:55 -0700 Cc: Peter Zijlstra , Linus Torvalds , Linux List Kernel Mailing , Ingo Molnar , Andrew Morton , Andy Lutomirski , Nicolai Stange , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , the arch/x86 maintainers , Josh Poimboeuf , Jiri Kosina , Miroslav Benes , Petr Mladek , Joe Lawrence , Shuah Khan , Konrad Rzeszutek Wilk , Tim Chen , Sebastian Andrzej Siewior , Mimi Zohar , Juergen Gross , Nick Desaulniers , Nayna Jain , Masahiro Yamada , Joerg Roedel , "open list:KERNEL SELFTEST FRAMEWORK" , stable Content-Transfer-Encoding: quoted-printable Message-Id: <2045370D-38D8-406C-9E94-C1D483E232C9@amacapital.net> References: <20190501203152.397154664@goodmis.org> <20190501232412.1196ef18@oasis.local.home> <20190502162133.GX2623@hirez.programming.kicks-ass.net> <20190502181811.GY2623@hirez.programming.kicks-ass.net> <20190502202146.GZ2623@hirez.programming.kicks-ass.net> <20190502185225.0cdfc8bc@gandalf.local.home> <20190502193129.664c5b2e@gandalf.local.home> <20190502195052.0af473cf@gandalf.local.home> <20190503092959.GB2623@hirez.programming.kicks-ass.net> <20190503092247.20cc1ff0@gandalf.local.home> To: Steven Rostedt Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On May 3, 2019, at 6:22 AM, Steven Rostedt wrote: >=20 > On Fri, 3 May 2019 11:29:59 +0200 > Peter Zijlstra wrote: >=20 >=20 >> OMG, WTF, ARGH... That code is fsck'ing horrible. I'd almost argue to >> always do the INT3 thing, just to avoid games like that. >=20 > Hehe, that's almost the exact same thoughts I had when seeing this > code ;-) >=20 >>=20 >> That said; for normal traps ®s->sp is indeed the previous context -- >> if it doesn't fall off the stack. Your hack detects the regular INT3 >> frame. Howver if regs->sp has been modified (int3_emulate_push, for >> example) your detectoring comes unstuck. >=20 > Yep. I realized the issue as well. But wanted to make sure this did > work when sp wasn't changed. >=20 >>=20 >> Now, it is rather unlikely these two code paths interact, but just to be >> safe, something like so might be more reliable: >>=20 >>=20 >> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c >> index 4b8ee05dd6ad..aceaad0cc9a9 100644 >> --- a/arch/x86/kernel/ptrace.c >> +++ b/arch/x86/kernel/ptrace.c >> @@ -163,6 +163,9 @@ static inline bool invalid_selector(u16 value) >> * stack pointer we fall back to regs as stack if no previous stack >> * exists. >> * >> + * There is a special case for INT3, there we construct a full pt_regs >> + * environment. We can detect this case by a high bit in regs->cs >> + * >> * This is valid only for kernel mode traps. >> */ >> unsigned long kernel_stack_pointer(struct pt_regs *regs) >> @@ -171,6 +174,9 @@ unsigned long kernel_stack_pointer(struct pt_regs *re= gs) >> unsigned long sp =3D (unsigned long)®s->sp; >> u32 *prev_esp; >>=20 >> + if (regs->__csh & (1 << 13)) /* test CS_FROM_INT3 */ >> + return regs->sp; >> + >=20 > Thanks, I was looking into doing something like this (setting a flag in > the int3 code), but didn't have the time to see the best way to do this. >=20 > I'll add this version of the code and run it through my tests. >=20 > -- Steve >=20 >> if (context =3D=3D (sp & ~(THREAD_SIZE - 1))) >> return sp; >>=20 >> --- a/arch/x86/entry/entry_32.S >> +++ b/arch/x86/entry/entry_32.S >> @@ -388,6 +388,7 @@ >>=20 >> #define CS_FROM_ENTRY_STACK (1 << 31) >> #define CS_FROM_USER_CR3 (1 << 30) >> +#define CS_FROM_INT3 (1 << 29) >>=20 >> .macro SWITCH_TO_KERNEL_STACK >>=20 >> @@ -1515,6 +1516,9 @@ ENTRY(int3) >>=20 >> add $16, 12(%esp) # point sp back at the previous context >>=20 >> + andl $0x0000ffff, 4(%esp) >> + orl $CS_FROM_INT3, 4(%esp) >> + >> pushl $-1 # orig_eax; mark as interrupt >>=20 >> SAVE_ALL >=20 So here=E2=80=99s a somewhat nutty suggestion: how about we tweak the 32-bit= entry code to emulate the sane 64-bit frame, not just for int3 but always? = Basically, the entry asm for entries from kernel mode would do, roughly: push $0 ;dummy for call emulation push %ss push $0 ;a dummy for ESP push 3*4(%esp) ;EFLAGS push 3*4(%esp) ;CS push 3*4(%esp) ;EIP push %rax lea 7*4(%esp), %rax mov %rax, 4*4(%esp) ;ESP And the exit asm would do a little dance to write EFLAGS, CS, and EIP to the= right spot, then load ESP-3*4 into %esp and do IRET. Now the annoying kernel_stack_pointer() hack can just go away, since regs->s= p is always correct! I probably screwed up some arithmetic there, but it=E2=80=99s the idea that c= ounts :) From mboxrd@z Thu Jan 1 00:00:00 1970 From: luto at amacapital.net (Andy Lutomirski) Date: Fri, 3 May 2019 09:20:55 -0700 Subject: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions In-Reply-To: <20190503092247.20cc1ff0@gandalf.local.home> References: <20190501203152.397154664@goodmis.org> <20190501232412.1196ef18@oasis.local.home> <20190502162133.GX2623@hirez.programming.kicks-ass.net> <20190502181811.GY2623@hirez.programming.kicks-ass.net> <20190502202146.GZ2623@hirez.programming.kicks-ass.net> <20190502185225.0cdfc8bc@gandalf.local.home> <20190502193129.664c5b2e@gandalf.local.home> <20190502195052.0af473cf@gandalf.local.home> <20190503092959.GB2623@hirez.programming.kicks-ass.net> <20190503092247.20cc1ff0@gandalf.local.home> Message-ID: <2045370D-38D8-406C-9E94-C1D483E232C9@amacapital.net> > On May 3, 2019, at 6:22 AM, Steven Rostedt wrote: > > On Fri, 3 May 2019 11:29:59 +0200 > Peter Zijlstra wrote: > > >> OMG, WTF, ARGH... That code is fsck'ing horrible. I'd almost argue to >> always do the INT3 thing, just to avoid games like that. > > Hehe, that's almost the exact same thoughts I had when seeing this > code ;-) > >> >> That said; for normal traps ®s->sp is indeed the previous context -- >> if it doesn't fall off the stack. Your hack detects the regular INT3 >> frame. Howver if regs->sp has been modified (int3_emulate_push, for >> example) your detectoring comes unstuck. > > Yep. I realized the issue as well. But wanted to make sure this did > work when sp wasn't changed. > >> >> Now, it is rather unlikely these two code paths interact, but just to be >> safe, something like so might be more reliable: >> >> >> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c >> index 4b8ee05dd6ad..aceaad0cc9a9 100644 >> --- a/arch/x86/kernel/ptrace.c >> +++ b/arch/x86/kernel/ptrace.c >> @@ -163,6 +163,9 @@ static inline bool invalid_selector(u16 value) >> * stack pointer we fall back to regs as stack if no previous stack >> * exists. >> * >> + * There is a special case for INT3, there we construct a full pt_regs >> + * environment. We can detect this case by a high bit in regs->cs >> + * >> * This is valid only for kernel mode traps. >> */ >> unsigned long kernel_stack_pointer(struct pt_regs *regs) >> @@ -171,6 +174,9 @@ unsigned long kernel_stack_pointer(struct pt_regs *regs) >> unsigned long sp = (unsigned long)®s->sp; >> u32 *prev_esp; >> >> + if (regs->__csh & (1 << 13)) /* test CS_FROM_INT3 */ >> + return regs->sp; >> + > > Thanks, I was looking into doing something like this (setting a flag in > the int3 code), but didn't have the time to see the best way to do this. > > I'll add this version of the code and run it through my tests. > > -- Steve > >> if (context == (sp & ~(THREAD_SIZE - 1))) >> return sp; >> >> --- a/arch/x86/entry/entry_32.S >> +++ b/arch/x86/entry/entry_32.S >> @@ -388,6 +388,7 @@ >> >> #define CS_FROM_ENTRY_STACK (1 << 31) >> #define CS_FROM_USER_CR3 (1 << 30) >> +#define CS_FROM_INT3 (1 << 29) >> >> .macro SWITCH_TO_KERNEL_STACK >> >> @@ -1515,6 +1516,9 @@ ENTRY(int3) >> >> add $16, 12(%esp) # point sp back at the previous context >> >> + andl $0x0000ffff, 4(%esp) >> + orl $CS_FROM_INT3, 4(%esp) >> + >> pushl $-1 # orig_eax; mark as interrupt >> >> SAVE_ALL > So here’s a somewhat nutty suggestion: how about we tweak the 32-bit entry code to emulate the sane 64-bit frame, not just for int3 but always? Basically, the entry asm for entries from kernel mode would do, roughly: push $0 ;dummy for call emulation push %ss push $0 ;a dummy for ESP push 3*4(%esp) ;EFLAGS push 3*4(%esp) ;CS push 3*4(%esp) ;EIP push %rax lea 7*4(%esp), %rax mov %rax, 4*4(%esp) ;ESP And the exit asm would do a little dance to write EFLAGS, CS, and EIP to the right spot, then load ESP-3*4 into %esp and do IRET. Now the annoying kernel_stack_pointer() hack can just go away, since regs->sp is always correct! I probably screwed up some arithmetic there, but it’s the idea that counts :) From mboxrd@z Thu Jan 1 00:00:00 1970 From: luto@amacapital.net (Andy Lutomirski) Date: Fri, 3 May 2019 09:20:55 -0700 Subject: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions In-Reply-To: <20190503092247.20cc1ff0@gandalf.local.home> References: <20190501203152.397154664@goodmis.org> <20190501232412.1196ef18@oasis.local.home> <20190502162133.GX2623@hirez.programming.kicks-ass.net> <20190502181811.GY2623@hirez.programming.kicks-ass.net> <20190502202146.GZ2623@hirez.programming.kicks-ass.net> <20190502185225.0cdfc8bc@gandalf.local.home> <20190502193129.664c5b2e@gandalf.local.home> <20190502195052.0af473cf@gandalf.local.home> <20190503092959.GB2623@hirez.programming.kicks-ass.net> <20190503092247.20cc1ff0@gandalf.local.home> Message-ID: <2045370D-38D8-406C-9E94-C1D483E232C9@amacapital.net> Content-Type: text/plain; charset="UTF-8" Message-ID: <20190503162055.tF7IcHf8PD5USPiOtpZNuj9xhHyhp7pRaS9_SQW4jVs@z> > On May 3, 2019,@6:22 AM, Steven Rostedt wrote: > > On Fri, 3 May 2019 11:29:59 +0200 > Peter Zijlstra wrote: > > >> OMG, WTF, ARGH... That code is fsck'ing horrible. I'd almost argue to >> always do the INT3 thing, just to avoid games like that. > > Hehe, that's almost the exact same thoughts I had when seeing this > code ;-) > >> >> That said; for normal traps ®s->sp is indeed the previous context -- >> if it doesn't fall off the stack. Your hack detects the regular INT3 >> frame. Howver if regs->sp has been modified (int3_emulate_push, for >> example) your detectoring comes unstuck. > > Yep. I realized the issue as well. But wanted to make sure this did > work when sp wasn't changed. > >> >> Now, it is rather unlikely these two code paths interact, but just to be >> safe, something like so might be more reliable: >> >> >> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c >> index 4b8ee05dd6ad..aceaad0cc9a9 100644 >> --- a/arch/x86/kernel/ptrace.c >> +++ b/arch/x86/kernel/ptrace.c >> @@ -163,6 +163,9 @@ static inline bool invalid_selector(u16 value) >> * stack pointer we fall back to regs as stack if no previous stack >> * exists. >> * >> + * There is a special case for INT3, there we construct a full pt_regs >> + * environment. We can detect this case by a high bit in regs->cs >> + * >> * This is valid only for kernel mode traps. >> */ >> unsigned long kernel_stack_pointer(struct pt_regs *regs) >> @@ -171,6 +174,9 @@ unsigned long kernel_stack_pointer(struct pt_regs *regs) >> unsigned long sp = (unsigned long)®s->sp; >> u32 *prev_esp; >> >> + if (regs->__csh & (1 << 13)) /* test CS_FROM_INT3 */ >> + return regs->sp; >> + > > Thanks, I was looking into doing something like this (setting a flag in > the int3 code), but didn't have the time to see the best way to do this. > > I'll add this version of the code and run it through my tests. > > -- Steve > >> if (context == (sp & ~(THREAD_SIZE - 1))) >> return sp; >> >> --- a/arch/x86/entry/entry_32.S >> +++ b/arch/x86/entry/entry_32.S >> @@ -388,6 +388,7 @@ >> >> #define CS_FROM_ENTRY_STACK (1 << 31) >> #define CS_FROM_USER_CR3 (1 << 30) >> +#define CS_FROM_INT3 (1 << 29) >> >> .macro SWITCH_TO_KERNEL_STACK >> >> @@ -1515,6 +1516,9 @@ ENTRY(int3) >> >> add $16, 12(%esp) # point sp back at the previous context >> >> + andl $0x0000ffff, 4(%esp) >> + orl $CS_FROM_INT3, 4(%esp) >> + >> pushl $-1 # orig_eax; mark as interrupt >> >> SAVE_ALL > So here’s a somewhat nutty suggestion: how about we tweak the 32-bit entry code to emulate the sane 64-bit frame, not just for int3 but always? Basically, the entry asm for entries from kernel mode would do, roughly: push $0 ;dummy for call emulation push %ss push $0 ;a dummy for ESP push 3*4(%esp) ;EFLAGS push 3*4(%esp) ;CS push 3*4(%esp) ;EIP push %rax lea 7*4(%esp), %rax mov %rax, 4*4(%esp) ;ESP And the exit asm would do a little dance to write EFLAGS, CS, and EIP to the right spot, then load ESP-3*4 into %esp and do IRET. Now the annoying kernel_stack_pointer() hack can just go away, since regs->sp is always correct! I probably screwed up some arithmetic there, but it’s the idea that counts :)