| Message ID | 20190228183220.15626-4-sudeep.holla@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | ptrace: consolidate PTRACE_SYSEMU handling and add support for arm64 | expand |
On Thu, Feb 28, 2019 at 10:32 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > Now that we have a new hook ptrace_syscall_enter that can be called from > syscall entry code and it handles PTRACE_SYSEMU in generic code, we > can do some cleanup using the same in syscall_trace_enter. > > Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > in syscall_slow_exit_work seems unnecessary. Let's remove the same. I wasn't cc'd on the whole series, so I can't easily review this. Do you have a test case to make sure that emulation still works? Are there adequate tests in tools/testing/selftests/x86? Do they still pass after this patch? --Andy
On 2019/3/1 2:32, Sudeep Holla wrote: > Now that we have a new hook ptrace_syscall_enter that can be called from > syscall entry code and it handles PTRACE_SYSEMU in generic code, we > can do some cleanup using the same in syscall_trace_enter. > > Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > in syscall_slow_exit_work seems unnecessary. Let's remove the same. I think we should not change the logic here. Is so, it will double the report of syscall when PTRACE_SYSEMU_SINGLESTEP is enabled. > > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > arch/x86/entry/common.c | 22 ++++------------------ > 1 file changed, 4 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c > index 7bc105f47d21..36457c1f87d2 100644 > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -70,22 +70,16 @@ static long syscall_trace_enter(struct pt_regs *regs) > > struct thread_info *ti = current_thread_info(); > unsigned long ret = 0; > -bool emulated = false; > u32 work; > > if (IS_ENABLED(CONFIG_DEBUG_ENTRY)) > BUG_ON(regs != task_pt_regs(current)); > > -work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY; > - > -if (unlikely(work & _TIF_SYSCALL_EMU)) > -emulated = true; > - > -if ((emulated || (work & _TIF_SYSCALL_TRACE)) && > - tracehook_report_syscall_entry(regs)) > +if (unlikely(ptrace_syscall_enter(regs))) > return -1L; > > -if (emulated) > +work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY; > +if ((work & _TIF_SYSCALL_TRACE) && tracehook_report_syscall_entry(regs)) > return -1L; > > #ifdef CONFIG_SECCOMP > @@ -227,15 +221,7 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags) > if (cached_flags & _TIF_SYSCALL_TRACEPOINT) > trace_sys_exit(regs, regs->ax); > > -/* > - * If TIF_SYSCALL_EMU is set, we only get here because of > - * TIF_SINGLESTEP (i.e. this is PTRACE_SYSEMU_SINGLESTEP). > - * We already reported this syscall instruction in > - * syscall_trace_enter(). > - */ > -step = unlikely( > -(cached_flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) > -== _TIF_SINGLESTEP); > +step = unlikely((cached_flags & _TIF_SINGLESTEP)); > if (step || cached_flags & _TIF_SYSCALL_TRACE) > tracehook_report_syscall_exit(regs, step); > } > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Sat, Mar 02, 2019 at 05:11:40PM -0800, Andy Lutomirski wrote: > On Thu, Feb 28, 2019 at 10:32 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > Now that we have a new hook ptrace_syscall_enter that can be called from > > syscall entry code and it handles PTRACE_SYSEMU in generic code, we > > can do some cleanup using the same in syscall_trace_enter. > > > > Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > > in syscall_slow_exit_work seems unnecessary. Let's remove the same. > > I wasn't cc'd on the whole series, so I can't easily review this. Do > you have a test case to make sure that emulation still works? Are > there adequate tests in tools/testing/selftests/x86? Do they still > pass after this patch? > I will ensure you are cc-ed on the whole threads, sorry for missing. I remember seeing some selftests, but I haven't run them yet. -- Regards, Sudeep
On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: > On 2019/3/1 2:32, Sudeep Holla wrote: > > Now that we have a new hook ptrace_syscall_enter that can be called from > > syscall entry code and it handles PTRACE_SYSEMU in generic code, we > > can do some cleanup using the same in syscall_trace_enter. > > > > Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > > in syscall_slow_exit_work seems unnecessary. Let's remove the same. > > I think we should not change the logic here. Is so, it will double the report of syscall > when PTRACE_SYSEMU_SINGLESTEP is enabled. > I don't think that should happen, but I may be missing something. Can you explain how ? -- Regards, Sudeep
On 2019/3/4 18:12, Sudeep Holla wrote: > On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: >> On 2019/3/1 2:32, Sudeep Holla wrote: >>> Now that we have a new hook ptrace_syscall_enter that can be called from >>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we >>> can do some cleanup using the same in syscall_trace_enter. >>> >>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP >>> in syscall_slow_exit_work seems unnecessary. Let's remove the same. >> >> I think we should not change the logic here. Is so, it will double the report of syscall >> when PTRACE_SYSEMU_SINGLESTEP is enabled. >> > > I don't think that should happen, but I may be missing something. > Can you explain how ? > > -- > Regards, > Sudeep > When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP) at the entry of a system call, no need to report at the exit of a system call. Regards, Haibo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
(I thought I had sent this email, last Tuesday itself, but saw this in my draft today, something went wrong, sorry for the delay) On Tue, Mar 05, 2019 at 02:14:47AM +0000, Haibo Xu (Arm Technology China) wrote: > On 2019/3/4 18:12, Sudeep Holla wrote: > > On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: > >> On 2019/3/1 2:32, Sudeep Holla wrote: > >>> Now that we have a new hook ptrace_syscall_enter that can be called from > >>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we > >>> can do some cleanup using the same in syscall_trace_enter. > >>> > >>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > >>> in syscall_slow_exit_work seems unnecessary. Let's remove the same. > >> > >> I think we should not change the logic here. Is so, it will double the report of syscall > >> when PTRACE_SYSEMU_SINGLESTEP is enabled. > >> > > > > I don't think that should happen, but I may be missing something. > > Can you explain how ? > > > > When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and > _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP) > at the entry of a system call, no need to report at the exit of a system > call. > Sorry, but I still not get it, we have: step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP); For me, this is same as: step = ((flags & _TIF_SINGLESTEP) == _TIF_SINGLESTEP) or if (flags & _TIF_SINGLESTEP) step = true; So when PTRACE_SYSEMU_SINGLESTEP, _TIF_SYSCALL_EMU and _TIF_SINGLESTEP are set and step evaluates to true. So dropping _TIF_SYSCALL_EMU here should be fine. Am I still missing something ? -- Regards, Sudeep
On 2019/3/12 2:34, Sudeep Holla wrote: > (I thought I had sent this email, last Tuesday itself, but saw this in my > draft today, something went wrong, sorry for the delay) > > On Tue, Mar 05, 2019 at 02:14:47AM +0000, Haibo Xu (Arm Technology China) wrote: >> On 2019/3/4 18:12, Sudeep Holla wrote: >>> On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: >>>> On 2019/3/1 2:32, Sudeep Holla wrote: >>>>> Now that we have a new hook ptrace_syscall_enter that can be called from >>>>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we >>>>> can do some cleanup using the same in syscall_trace_enter. >>>>> >>>>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP >>>>> in syscall_slow_exit_work seems unnecessary. Let's remove the same. >>>> >>>> I think we should not change the logic here. Is so, it will double the report of syscall >>>> when PTRACE_SYSEMU_SINGLESTEP is enabled. >>>> >>> >>> I don't think that should happen, but I may be missing something. >>> Can you explain how ? >>> >> >> When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and >> _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP) >> at the entry of a system call, no need to report at the exit of a system >> call. >> > Sorry, but I still not get it, we have: > > step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP); > > For me, this is same as: > step = ((flags & _TIF_SINGLESTEP) == _TIF_SINGLESTEP) > or > if (flags & _TIF_SINGLESTEP) > step = true; > I don't think so! As I mentioned in the last email loop, when PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and _TIF_SINGLESTEP flags are set, in which case the step should be "false" for the old logic. But with the new logic, the step is "true". > So when PTRACE_SYSEMU_SINGLESTEP, _TIF_SYSCALL_EMU and _TIF_SINGLESTEP > are set and step evaluates to true. > > So dropping _TIF_SYSCALL_EMU here should be fine. Am I still missing > something ? > > -- > Regards, > Sudeep > For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send SIGTRAP) at the entry of a system call, no need to report at the exit of a system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special case(PTRACE_SYSEMU_SINGLESTEP). Another way to make sure the logic is fine, you can run some tests with respect to both logic, and to check whether they have the same behavior. Regards, Haibo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Mon, Mar 11, 2019 at 6:35 PM Haibo Xu (Arm Technology China) <Haibo.Xu@arm.com> wrote: > > On 2019/3/12 2:34, Sudeep Holla wrote: > > (I thought I had sent this email, last Tuesday itself, but saw this in my > > draft today, something went wrong, sorry for the delay) > > > > On Tue, Mar 05, 2019 at 02:14:47AM +0000, Haibo Xu (Arm Technology China) wrote: > >> On 2019/3/4 18:12, Sudeep Holla wrote: > >>> On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: > >>>> On 2019/3/1 2:32, Sudeep Holla wrote: > >>>>> Now that we have a new hook ptrace_syscall_enter that can be called from > >>>>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we > >>>>> can do some cleanup using the same in syscall_trace_enter. > >>>>> > >>>>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > >>>>> in syscall_slow_exit_work seems unnecessary. Let's remove the same. > >>>> > >>>> I think we should not change the logic here. Is so, it will double the report of syscall > >>>> when PTRACE_SYSEMU_SINGLESTEP is enabled. > >>>> > >>> > >>> I don't think that should happen, but I may be missing something. > >>> Can you explain how ? > >>> > >> > >> When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and > >> _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP) > >> at the entry of a system call, no need to report at the exit of a system > >> call. > >> > > Sorry, but I still not get it, we have: > > > > step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP); > > > > For me, this is same as: > > step = ((flags & _TIF_SINGLESTEP) == _TIF_SINGLESTEP) > > or > > if (flags & _TIF_SINGLESTEP) > > step = true; > > > > I don't think so! As I mentioned in the last email loop, when PTRACE_SYSEMU_SINGLESTEP > is enabled, both the _TIF_SYSCALL_EMU and _TIF_SINGLESTEP flags are set, in which case > the step should be "false" for the old logic. But with the new logic, the step is "true". > > > So when PTRACE_SYSEMU_SINGLESTEP, _TIF_SYSCALL_EMU and _TIF_SINGLESTEP > > are set and step evaluates to true. > > > > So dropping _TIF_SYSCALL_EMU here should be fine. Am I still missing > > something ? > > > > -- > > Regards, > > Sudeep > > > > For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send SIGTRAP) > at the entry of a system call, no need to report at the exit of a system call.That's > why the old logic-{step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} > here try to filter out the special case(PTRACE_SYSEMU_SINGLESTEP). > > Another way to make sure the logic is fine, you can run some tests with respect to both logic, > and to check whether they have the same behavior. tools/testing/selftests/x86/ptrace_syscall.c has a test intended to exercise this. Can one of you either confirm that it does exercise it and that it still passes or can you improve the test? Thanks, Andy
On Tue, Mar 12, 2019 at 01:34:44AM +0000, Haibo Xu (Arm Technology China) wrote: > On 2019/3/12 2:34, Sudeep Holla wrote: > > (I thought I had sent this email, last Tuesday itself, but saw this in my > > draft today, something went wrong, sorry for the delay) > > > > On Tue, Mar 05, 2019 at 02:14:47AM +0000, Haibo Xu (Arm Technology China) wrote: > >> On 2019/3/4 18:12, Sudeep Holla wrote: > >>> On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote: > >>>> On 2019/3/1 2:32, Sudeep Holla wrote: > >>>>> Now that we have a new hook ptrace_syscall_enter that can be called from > >>>>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we > >>>>> can do some cleanup using the same in syscall_trace_enter. > >>>>> > >>>>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP > >>>>> in syscall_slow_exit_work seems unnecessary. Let's remove the same. > >>>> > >>>> I think we should not change the logic here. Is so, it will double the report of syscall > >>>> when PTRACE_SYSEMU_SINGLESTEP is enabled. > >>>> > >>> > >>> I don't think that should happen, but I may be missing something. > >>> Can you explain how ? > >>> > >> > >> When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and > >> _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP) > >> at the entry of a system call, no need to report at the exit of a system > >> call. > >> > > Sorry, but I still not get it, we have: > > > > step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP); > > > > For me, this is same as: > > step = ((flags & _TIF_SINGLESTEP) == _TIF_SINGLESTEP) > > or > > if (flags & _TIF_SINGLESTEP) > > step = true; > > > > I don't think so! As I mentioned in the last email loop, when > PTRACE_SYSEMU_SINGLESTE is enabled, both the _TIF_SYSCALL_EMU and > _TIF_SINGLESTEP flags are set, in which case the step should be "false" for > the old logic. But with the new logic, the step is "true". > Ah right, sorry I missed that. > > So when PTRACE_SYSEMU_SINGLESTEP, _TIF_SYSCALL_EMU and _TIF_SINGLESTEP > > are set and step evaluates to true. > > > > So dropping _TIF_SYSCALL_EMU here should be fine. Am I still missing > > something ? > > > > -- > > Regards, > > Sudeep > > > > For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send > SIGTRAP) at the entry of a system call, no need to report at the exit of a > system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP | > _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special > case(PTRACE_SYSEMU_SINGLESTEP). > Understood > Another way to make sure the logic is fine, you can run some tests with > respect to both logic, and to check whether they have the same behavior. > I did run selftests after Andy Lutomirski pointed out. Nothing got flagged, I haven't looked at the tests themselves yet, but it clearly misses this case. -- Regards, Sudeep
On Mon, Mar 11, 2019 at 08:04:39PM -0700, Andy Lutomirski wrote: > On Mon, Mar 11, 2019 at 6:35 PM Haibo Xu (Arm Technology China) > <Haibo.Xu@arm.com> wrote: > > [...] > > For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send > > SIGTRAP) at the entry of a system call, no need to report at the exit of a > > system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP | > > _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special > > case(PTRACE_SYSEMU_SINGLESTEP). > > > > Another way to make sure the logic is fine, you can run some tests with > > respect to both logic, and to check whether they have the same behavior. > > tools/testing/selftests/x86/ptrace_syscall.c has a test intended to > exercise this. Can one of you either confirm that it does exercise it > and that it still passes or can you improve the test? > I did run the tests which didn't flag anything. I haven't looked at the details of test implementation, but seem to miss this case. I will see what can be improved(if it's possible). Also I think single_step_syscall is the one I need to look for this particular one. Both single_step_syscall ptrace_syscall reported no errors. -- Regards, Sudeep
On 2019/3/12 20:09, Sudeep Holla wrote: > On Mon, Mar 11, 2019 at 08:04:39PM -0700, Andy Lutomirski wrote: >> On Mon, Mar 11, 2019 at 6:35 PM Haibo Xu (Arm Technology China) >> <Haibo.Xu@arm.com> wrote: >>> > > [...] > >>> For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send >>> SIGTRAP) at the entry of a system call, no need to report at the exit of a >>> system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP | >>> _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special >>> case(PTRACE_SYSEMU_SINGLESTEP). >>> >>> Another way to make sure the logic is fine, you can run some tests with >>> respect to both logic, and to check whether they have the same behavior. >> >> tools/testing/selftests/x86/ptrace_syscall.c has a test intended to >> exercise this. Can one of you either confirm that it does exercise it >> and that it still passes or can you improve the test? >> > I did run the tests which didn't flag anything. I haven't looked at the > details of test implementation, but seem to miss this case. I will see > what can be improved(if it's possible). Also I think single_step_syscall > is the one I need to look for this particular one. Both single_step_syscall > ptrace_syscall reported no errors. > > -- > Regards, > Sudeep > Since ptrace() system call do have so many request type, I'm not sure whether the test cases have covered all of that. But here we'd better make sure the PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP requests are work correctly. May be you can verify them with tests from Bin Lu(bin.lu@arm.com). Regards, Haibo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Wed, Mar 13, 2019 at 01:03:18AM +0000, Haibo Xu (Arm Technology China) wrote: [...] > Since ptrace() system call do have so many request type, I'm not sure > whether the test cases have covered all of that. But here we'd better make > sure the PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP requests are work > correctly. May be you can verify them with tests from Bin Lu(bin.lu@arm.com). Sure happy to try them. Can you point me to them ? I did end up writing few more tests. -- Regards, Sudeep
On 2019/3/14 18:51, Sudeep Holla wrote: > On Wed, Mar 13, 2019 at 01:03:18AM +0000, Haibo Xu (Arm Technology China) wrote: > [...] > >> Since ptrace() system call do have so many request type, I'm not sure >> whether the test cases have covered all of that. But here we'd better make >> sure the PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP requests are work >> correctly. May be you can verify them with tests from Bin Lu(bin.lu@arm.com). > > Sure happy to try them. Can you point me to them ? > I did end up writing few more tests. > > -- > Regards, > Sudeep > You can get them from Steve Capper. BTW, I also have a program to verify the PTRACE_SYSEMU function, and will send to you in a separate email loop. Regards, Haibo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 7bc105f47d21..36457c1f87d2 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -70,22 +70,16 @@ static long syscall_trace_enter(struct pt_regs *regs) struct thread_info *ti = current_thread_info(); unsigned long ret = 0; - bool emulated = false; u32 work; if (IS_ENABLED(CONFIG_DEBUG_ENTRY)) BUG_ON(regs != task_pt_regs(current)); - work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY; - - if (unlikely(work & _TIF_SYSCALL_EMU)) - emulated = true; - - if ((emulated || (work & _TIF_SYSCALL_TRACE)) && - tracehook_report_syscall_entry(regs)) + if (unlikely(ptrace_syscall_enter(regs))) return -1L; - if (emulated) + work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY; + if ((work & _TIF_SYSCALL_TRACE) && tracehook_report_syscall_entry(regs)) return -1L; #ifdef CONFIG_SECCOMP @@ -227,15 +221,7 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags) if (cached_flags & _TIF_SYSCALL_TRACEPOINT) trace_sys_exit(regs, regs->ax); - /* - * If TIF_SYSCALL_EMU is set, we only get here because of - * TIF_SINGLESTEP (i.e. this is PTRACE_SYSEMU_SINGLESTEP). - * We already reported this syscall instruction in - * syscall_trace_enter(). - */ - step = unlikely( - (cached_flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) - == _TIF_SINGLESTEP); + step = unlikely((cached_flags & _TIF_SINGLESTEP)); if (step || cached_flags & _TIF_SYSCALL_TRACE) tracehook_report_syscall_exit(regs, step); }
Now that we have a new hook ptrace_syscall_enter that can be called from syscall entry code and it handles PTRACE_SYSEMU in generic code, we can do some cleanup using the same in syscall_trace_enter. Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP in syscall_slow_exit_work seems unnecessary. Let's remove the same. Cc: Andy Lutomirski <luto@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- arch/x86/entry/common.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) -- 2.17.1