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=-3.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no 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 D309BC433B4 for ; Thu, 22 Apr 2021 16:44:33 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3171F61422 for ; Thu, 22 Apr 2021 16:44:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3171F61422 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; 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=x+G6YQWnNzeH+ASxtveTstmPKC1KVlWpfsSMKiRwNDk=; b=c5YDNaxx7Rvme9Wtqxd+9O5vN zatYtKwitAwsaKdMng5A0oivlO2EmVtOz2rEiW5pi2iiy1xJqF2LsWI3vClS4ITr1r2ICWUw0Z++u Gzoxp701sDO5x0wBIeTnSBpQ+FSD16C/lW1Vk6Kb+tpVeH8LcPgeDPuDfSX8sBDrH/i22YX1/DFlF 66+ED+T9Dn8afDbIVDjsy29G/EHG3TAhEHUmJaouUkgasLs98Krh/6T3AL5QekWzLBJiX+1eXCKwQ j8DfB1vhgAPpWyxpmeHcULLFbsRZEWE8IMEWaeNiWLbOVfxcUA9Fw48ObpF7hkh9yE2h79mGAOVYP +vDROS+hA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lZcPa-00H6vi-HD; Thu, 22 Apr 2021 16:42:42 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lZcPX-00H6vT-SZ for linux-arm-kernel@desiato.infradead.org; Thu, 22 Apr 2021 16:42:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=1I97ZvDUOMzOrn2xY6UY5CSeYPfOQO1cYrWPsN3Hg2c=; b=ykgzYSc1jdNmy5abji5u81KLto 7RYKFuYcfA+dNHuYLxgd9PREB0DV4IdF5xXgkEDBGRQdAnSw/TGeciapG1ExmMHMQ4sY44CLN9xNL lSE0AGYbTKgY1BniQRhn+ou2egpwXlNZXg0TxDHC9dCnE5QMZiOHV3RGAcFuchluPehLEtGkuAg57 wXJCMTl2cIAofe7J9wwTuVk7mr7xwwt1tUMcPF3hgPEZ5lMNsmGuag/q63f0QLzsodLFttm2de8wA Vmm5xL0vHZ4UGDtIrCOhU+oNAIv9l0zARNkd34ELSmuZH0uo9vRVKMrXGbokYSMoEZxLih3+TQ0B0 AO5gOJOQ==; Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lZcPU-00Dr7D-PG for linux-arm-kernel@lists.infradead.org; Thu, 22 Apr 2021 16:42:38 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 63BC011D4; Thu, 22 Apr 2021 09:42:32 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.22.241]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D7E033F73B; Thu, 22 Apr 2021 09:42:30 -0700 (PDT) Date: Thu, 22 Apr 2021 17:42:28 +0100 From: Mark Rutland To: Will Deacon Cc: Catalin Marinas , He Zhe , oleg@redhat.com, linux-arm-kernel@lists.infradead.org, paul@paul-moore.com, eparis@redhat.com, linux-audit@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] arm64: ptrace: Add is_syscall_success to handle compat Message-ID: <20210422164228.GD66392@C02TD0UTHF1T.local> References: <20210416075533.7720-1-zhe.he@windriver.com> <20210416123322.GA23184@arm.com> <20210416133431.GA2303@C02TD0UTHF1T.local> <20210419121932.GA30004@willie-the-truck> <20210421171005.GA46949@C02TD0UTHF1T.local> <20210422160752.GA2214@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210422160752.GA2214@willie-the-truck> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210422_094236_935244_323CB997 X-CRM114-Status: GOOD ( 38.80 ) 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 Thu, Apr 22, 2021 at 05:07:53PM +0100, Will Deacon wrote: > On Wed, Apr 21, 2021 at 06:10:05PM +0100, Mark Rutland wrote: > > On Mon, Apr 19, 2021 at 01:19:33PM +0100, Will Deacon wrote: > > > On Fri, Apr 16, 2021 at 02:34:41PM +0100, Mark Rutland wrote: > > > > I think this is a problem we created for ourselves back in commit: > > > > > > > > 15956689a0e60aa0 ("arm64: compat: Ensure upper 32 bits of x0 are zero on syscall return) > > > > > > > > AFAICT, the perf regs samples are the only place this matters, since for > > > > ptrace the compat regs are implicitly truncated to compat_ulong_t, and > > > > audit expects the non-truncated return value. Other architectures don't > > > > truncate here, so I think we're setting ourselves up for a game of > > > > whack-a-mole to truncate and extend wherever we need to. > > > > > > > > Given that, I suspect it'd be better to do something like the below. > > > > > > > > Will, thoughts? > > > > > > I think perf is one example, but this is also visible to userspace via the > > > native ptrace interface and I distinctly remember needing this for some > > > versions of arm64 strace to work correctly when tracing compat tasks. > > > > FWIW, you've convinced me on your approach (more on that below), but > > when I went digging here this didn't seem to be exposed via ptrace -- > > for any task tracing a compat task, the GPRs are exposed via > > compat_gpr_{get,set}(), which always truncate to compat_ulong_t, giving > > the lower 32 bits. See task_user_regset_view() for where we get the > > regset. > > > > Am I missing something, or are you thinking of another issue you fixed > > at the same time? > > I think it may depend on whether strace pokes at the GPRs or instead issues > a PTRACE_GET_SYSCALL_INFO request but I've forgotten the details, > unfortunately. I do remember seeing an issue though, and it was only last > year. Ah; I hadn't spotted PTRACE_GET_SYSCALL_INFO, thanks for the pointer. I see that gets at the regs via syscall_get_arguments(), which doesn't truncate them. That makes me wonder whether x86 and others do the right thing here... > > > So I do think that clearing the upper bits on the return path is the right > > > approach, but it sounds like we need some more work to handle syscall(-1) > > > and audit (what exactly is the problem here after these patches have been > > > applied?) > > > > From digging a bit more, I think I agree, and I think these patches are > > sufficient for audit. I have some comments I'll leave separately. > > > > The remaining issues are wherever we assign a signed value to a compat > > GPR without explicit truncation. That'll leak via perf sampling the user > > regs, but I haven't managed to convince myself whether that causes any > > functional change in behaviour for audit, seccomp, or syscall tracing. > > > > Since we mostly use compat_ulong_t for intermediate values in compat > > code, it does look like this is only an issue for x0 where we assign an > > error value, e.g. the -ENOSYS case in el0_svc_common. I'll go see if I > > can find any more. > > > > With those fixed up we can remove the x0 truncation from entry.S, > > which'd be nice too. > > If we remove that then we should probably have a (debug?) check on the > return-to-user path just to make sure. I've hacked that up locally; I'd certainly like to run that along with some other sanity checks (e.g. PSTATE) while fuzzing, but agree that's probably not something to enable in any defconfig build. I'm happy to add that later as and when I remove the bit from entry.S. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel