From patchwork Tue May 26 22:38:24 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josh Stone X-Patchwork-Id: 6486161 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 83036C0020 for ; Tue, 26 May 2015 22:41:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 92309206B1 for ; Tue, 26 May 2015 22:41:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4D71420670 for ; Tue, 26 May 2015 22:41:10 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YxNUo-0004Q7-9K; Tue, 26 May 2015 22:38:50 +0000 Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YxNUk-0004Ly-Ja for linux-arm-kernel@lists.infradead.org; Tue, 26 May 2015 22:38:47 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 8B2B7BDD88; Tue, 26 May 2015 22:38:25 +0000 (UTC) Received: from [10.3.113.24] (ovpn-113-24.phx2.redhat.com [10.3.113.24]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4QMcOe9031312; Tue, 26 May 2015 18:38:25 -0400 Message-ID: <5564F5E0.7080108@redhat.com> Date: Tue, 26 May 2015 15:38:24 -0700 From: Josh Stone User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Russell King - ARM Linux Subject: Re: arm syscall fast path can miss a ptrace syscall-exit References: <5554F3E4.8020307@redhat.com> <20150514193553.GD2067@n2100.arm.linux.org.uk> <55550EBA.3050708@redhat.com> In-Reply-To: <55550EBA.3050708@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150526_153846_689361_190A748D X-CRM114-Status: GOOD ( 19.56 ) X-Spam-Score: -5.0 (-----) Cc: linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, Do you plan to commit this check for syscall flags? On 05/14/2015 02:08 PM, Josh Stone wrote: > On 05/14/2015 12:35 PM, Russell King - ARM Linux wrote: >> On Thu, May 14, 2015 at 12:13:40PM -0700, Josh Stone wrote: >>> I've discovered a case where both arm and arm64 will miss a ptrace >>> syscall-exit that they should report. If the syscall is entered without >>> TIF_SYSCALL_TRACE set, then it goes on the fast path. It's then >>> possible to have TIF_SYSCALL_TRACE added in the middle of the syscall, >>> but ret_fast_syscall doesn't check this flag again. >> >> Yes, we assume that if TIF_SYSCALL_TRACE was not set before the call, it >> isn't set after. That appears to be an invalid assumption. >> >> Here's a patch for ARM - untested atm. > > Thanks! The system I have at hand is arm64, so I made the similar > change in arch/arm64/kernel/entry.S, and this passes my test. FWIW, here's the arm64 change I tested following your example: >> There's still a possible hole - if we exit the syscall, then do "work" >> before returning (such as reschedling to another process), and _then_ >> have syscall tracing enabled, we won't trace the exit. I think that's >> acceptable as I see no difference between that and having restored >> state for userspace, and then immediately processing an interrupt and >> scheduling on the IRQ exit path. > > Yeah, I think that's fine. I don't think that hole is visible to > ptrace, at least, and other tracers already have to accept this > possibility anyway. > >> >> arch/arm/kernel/entry-common.S | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S >> index f8ccc21fa032..4e7f40c577e6 100644 >> --- a/arch/arm/kernel/entry-common.S >> +++ b/arch/arm/kernel/entry-common.S >> @@ -33,7 +33,9 @@ ret_fast_syscall: >> UNWIND(.fnstart ) >> UNWIND(.cantunwind ) >> disable_irq @ disable interrupts >> - ldr r1, [tsk, #TI_FLAGS] >> + ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing >> + tst r1, #_TIF_SYSCALL_WORK >> + bne __sys_trace_return >> tst r1, #_TIF_WORK_MASK >> bne fast_work_pending >> asm_trace_hardirqs_on >> >> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index 959fe8733560..a547a3e8a198 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -608,7 +608,9 @@ ENDPROC(cpu_switch_to) */ 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