All of lore.kernel.org
 help / color / mirror / Atom feed
From: jistone@redhat.com (Josh Stone)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: fix missing syscall trace exit
Date: Wed, 03 Jun 2015 13:03:49 -0700	[thread overview]
Message-ID: <556F5DA5.2050207@redhat.com> (raw)
In-Reply-To: <20150603095241.GD17581@arm.com>

On 06/03/2015 02:52 AM, Will Deacon wrote:
> On Wed, Jun 03, 2015 at 02:11:48AM +0100, Josh Stone wrote:
>> On 06/02/2015 06:01 PM, Josh Stone wrote:
>>>  ret_fast_syscall:
>>>  	disable_irq				// disable interrupts
>>> -	ldr	x1, [tsk, #TI_FLAGS]
>>> +	ldr	x1, [tsk, #TI_FLAGS]		// re-check for syscall tracing
>>> +	and	x2, x1, #_TIF_SYSCALL_WORK
>>> +	cbnz	x2, __sys_trace_return
>>>  	and	x2, x1, #_TIF_WORK_MASK
>>>  	cbnz	x2, fast_work_pending
>>>  	enable_step_tsk x1, x2
>>
>> I do have one concern about this, also in Russell's ARM patch.  Is it
>> really ok to branch to __sys_trace_return with interrupts disabled?
> 
> I think you're right to be concerned!
> 
>> I didn't hit any issue from that, but my testcase only exercises this
>> path once each run.  So that might have just been lucky not to hit any
>> gross scenario...
> 
> Did you try enabling all the audit stuff? It looks like that can call
> into the scheduler, so I think we should be running the tracing callbacks
> with interrupts enabled (and it looks like x86 do this on the exit path).

This particular path only applies if you entered the syscall *without*
any tracing, which is what makes it pretty much a oneshot.  You'd have
to arrange for audit enabling in the middle of a syscall to see it.  My
ptrace test is easier because working from PTRACE_EVENT_FORK is always
in the middle of the fork syscall.

But anyway, I agree interrupts should be enabled -- I'll work on this.
Then after __sys_trace_return jumps to ret_from_user, they'll be
disabled again.  Likewise for arm32 jumping to ret_slow_syscall.

  reply	other threads:[~2015-06-03 20:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-14 19:13 arm syscall fast path can miss a ptrace syscall-exit Josh Stone
2015-05-14 19:35 ` Russell King - ARM Linux
2015-05-14 21:08   ` Josh Stone
2015-05-26 22:38     ` Josh Stone
2015-05-28 10:37       ` Russell King - ARM Linux
2015-05-29 20:13         ` Josh Stone
2015-06-01 10:24           ` Will Deacon
2015-06-03  1:01             ` [PATCH] arm64: fix missing syscall trace exit Josh Stone
2015-06-03  1:11               ` Josh Stone
2015-06-03  9:52                 ` Will Deacon
2015-06-03 20:03                   ` Josh Stone [this message]
2015-06-04 10:06                 ` Russell King - ARM Linux
2015-06-04 17:14                   ` Josh Stone
2015-06-04 23:17                     ` Josh Stone
2015-06-05 15:38                       ` Will Deacon
2015-06-05 17:52                         ` Tom Lendacky
2015-06-05 21:28                         ` Josh Stone
2015-06-08 10:21                           ` Will Deacon
2015-06-08 16:37                             ` Josh Stone
2015-06-08 16:43                               ` Catalin Marinas
2015-06-23  0:08                   ` [PATCH] ARM: enable_irq before ret_fast_syscall tracing Josh Stone
2015-06-23  0:15                     ` Josh Stone

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=556F5DA5.2050207@redhat.com \
    --to=jistone@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.