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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C9E25C001DE for ; Thu, 10 Aug 2023 19:32:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=itS8HtVD+yIRk89bOsu7OOf4MV1q4JGMg8ovNd6f0ys=; b=tyg9psNsGta5St 7dN1bjf6UOEc4/DmJb/r2PMkH3rdt+OARFnAK9nskHnDIczqZ2o3QYp+NWYrhp3ZBWSpE7sJpVJ2N VmnUisby5Ik/Wcv1RG503PVpjZr/3TfQP1vAddxKVnJza0/KD91Nnet2tjoB3eVwtZtQJUTw3pn5y H/GpyjCGr7fEgQepYeYsXTdhhz6rZWcRbuPfDAYQkO3uqQ1Ud3hdPXOP0K3w+zANySlgO7Mtgtj20 5QwwXmc6DZ15o7CPJNbnQ2j3uOkVvqUJSEBcx99lq4YD2jsbqqhFRY11vGGGXU5ZbOHkq63GHRKU6 rgFIg+T6x+QEJ32KpExQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qUBOH-008YvN-0A; Thu, 10 Aug 2023 19:32:13 +0000 Received: from mail-pl1-x633.google.com ([2607:f8b0:4864:20::633]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qUBOE-008YuG-0O for linux-arm-kernel@lists.infradead.org; Thu, 10 Aug 2023 19:32:11 +0000 Received: by mail-pl1-x633.google.com with SMTP id d9443c01a7336-1bba54f7eefso17050715ad.1 for ; Thu, 10 Aug 2023 12:32:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1691695925; x=1692300725; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=n5BUcp4rF73339tUbdxChTzYXicpDGHwoyiHDjbgdIQ=; b=MUhRj4IulKry/rRZHaY3UgOWWMElBQrZl4hueG5jLdyTT2NIolBHKKjvU4Y0TzRsrV +wxsVbj+iSIqkXPRzCoYDK57C46cw7kTcbKwyUpB76DazMVmyYYiSy6GhVq+Dhfl6HN/ vUgOY4RX1nzfdz3uoA+58xgi8C9eqv2Wdwhi8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691695925; x=1692300725; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=n5BUcp4rF73339tUbdxChTzYXicpDGHwoyiHDjbgdIQ=; b=e2d+kvSCj0eWmsUwfSU11E3gvKwhaDh76By1QZI1MxJUR18DS8N5H0XrzrwAOtHCWI bzYuEBnvEGK/vz77l2BXAj5sNTYzt04G7Eaax1kdlgdWyX8pUsJ8mkeP7wVhJ+qfV6uU pjBNnYNswE7Qcd4YcZAkxD7tW9DJ21SMOSYM9lIhtYtgcAyxtuKvTPTdG+lBNYLX8yec 2ehK9ayX2Git1m/qn7XJcoB1OQTdAi2mYW6iJOMIxEK/w5wTY55kUQ5WXaLbWN5ln6og IZ0nrgUYpS5S1bIu6HjFPCznpwDksEY3D2MeRRchEY1KxIVAU0HHyCjAqE/Cid6Qe+Hk 2Adw== X-Gm-Message-State: AOJu0YxzqGvy9GFSTAf64AihNi2rXs/T2+qeQUtxFo3O14P2kWsG/TBY 79sPpFQqzFzyxjN3RYDz1Wmx+A== X-Google-Smtp-Source: AGHT+IEOUo5qUDRJcUued1hIUrxl+tnnYRC3515yaFsDbHi7mTOPBSy4Sn1t0M7Wa4EdXVSzgg3wqg== X-Received: by 2002:a05:6a20:431b:b0:13a:dd47:c31a with SMTP id h27-20020a056a20431b00b0013add47c31amr3971923pzk.20.1691695925514; Thu, 10 Aug 2023 12:32:05 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id k17-20020a637b51000000b005639da5a8e2sm1896326pgn.2.2023.08.10.12.32.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Aug 2023 12:32:04 -0700 (PDT) Date: Thu, 10 Aug 2023 12:32:04 -0700 From: Kees Cook To: Arnd Bergmann Cc: Russell King , Lecopzer Chen , Oleg Nesterov , linux-arm-kernel@lists.infradead.org, Linus Walleij , Russell King , linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH] ARM: ptrace: Restore syscall skipping and restart while tracing Message-ID: <202308101209.45CF7C6F80@keescook> References: <20230804071045.never.134-kees@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230810_123210_212856_5080ECD4 X-CRM114-Status: GOOD ( 33.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Aug 09, 2023 at 09:47:24PM +0200, Arnd Bergmann wrote: > On Fri, Aug 4, 2023, at 09:10, Kees Cook wrote: > > Since commit 4e57a4ddf6b0 ("ARM: 9107/1: syscall: always store > > thread_info->abi_syscall"), the seccomp selftests "syscall_errno", > > "syscall_faked", and "syscall_restart" have been broken. This was > > related to two issues: > > While it looks like my patch introduced both problems, it might > be better to split your fix into two bits. Okay, sounds good. > > - seccomp and PTRACE depend on using the special value of "-1" for > > skipping syscalls. This value wasn't working because it was getting > > masked by __NR_SYSCALL_MASK in both PTRACE_SET_SYSCALL and > > get_syscall_nr(). > > > Explicitly test for -1 in PTRACE_SET_SYSCALL and get_syscall_nr(), > > leaving it exposed when present, allowing tracers to skip syscalls > > again. > > This part looks good to me, at least it seems to be one of multiple > ways of doing this, depending on how we want to encode the > syscall skipping in the variable. > > > - the syscall entry label "local_restart" is used for resuming syscalls > > interrupted by signals, but the updated syscall number (in scno) was > > not being stored in current_thread_info()->abi_syscall, causing traced > > syscall restarting to fail. > > > > Move the AEABI-only assignment of current_thread_info()->abi_syscall > > after the "local_restart" label to allow tracers to survive syscall > > restarting. > > I'm not following exactly what you are doing here yet, but I suspect > this part is wrong: > > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > > index bcc4c9ec3aa4..08bd624e4c6f 100644 > > --- a/arch/arm/kernel/entry-common.S > > +++ b/arch/arm/kernel/entry-common.S > > @@ -246,8 +246,6 @@ ENTRY(vector_swi) > > bic scno, scno, #0xff000000 @ mask off SWI op-code > > str scno, [tsk, #TI_ABI_SYSCALL] > > eor scno, scno, #__NR_SYSCALL_BASE @ check OS number > > -#else > > - str scno, [tsk, #TI_ABI_SYSCALL] > > #endif > > /* > > * Reload the registers that may have been corrupted on entry to > > @@ -256,6 +254,9 @@ ENTRY(vector_swi) > > TRACE( ldmia sp, {r0 - r3} ) > > > > local_restart: > > +#if defined(CONFIG_AEABI) && !defined(CONFIG_OABI_COMPAT) > > + str scno, [tsk, #TI_ABI_SYSCALL] @ store scno for syscall restart > > +#endif > > ldr r10, [tsk, #TI_FLAGS] @ check for syscall tracing > > stmdb sp!, {r4, r5} @ push fifth and sixth args > > > > If the local_restart code has to store the syscall number > for an EABI-only kernel, wouldn't it have to also do this > for a kernel with OABI-only or OABI_COMPAT support? This is the part I wasn't sure about. Initially I was thinking it didn't matter because it's only a problem for a seccomp tracer, but I realize it might be exposed to a PTRACE tracer too. I was only able to test with EABI since seccomp is disabled for OABI_COMPAT. Anyway, syscall restart is done this way: movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE) Can a EABI call restart an OABI syscall? I think so? So maybe we just need to add: str scno, [tsk, #TI_ABI_SYSCALL] @ store scno for syscall restart after that instead of moving it like I did originally? Let me test that... -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel