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=-5.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham 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 195CCC433DF for ; Tue, 23 Jun 2020 14:40:43 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 D96E12053B for ; Tue, 23 Jun 2020 14:40:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="2fEyns9q" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D96E12053B Authentication-Results: mail.kernel.org; dmarc=none (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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=V2RnrZ5wYtpWwL1Hz0uineAmzNhmH7H78tMi811j2ro=; b=2fEyns9qPP66JOgkKIZ6b1n3O FY4d2HDBaJCmJhGYstRWmnXEMRapGZGXVc+lzXY2r3NKNsdMJQvt7cZxYz3LaDWyyUFz7FrLB3KRa w9aIjt7v8Wti7fPOwpcNEDN7l4Dd7NWi6+syJpLluS20oKyM2ytN7h3TwIDz+CIcNOspevTLAma96 Sb8v6tlE5Li1VRKXmOklgpst5itE9cFrR0LSYjIHLx6QsacLn3ShS0uzii0vUt6d0xSb6Jmln/o9M lNswqijfbK4gvYVTmMzyzi1YwWhgCwwHv7WEnQuFh7Isy4rDnCVNMAO9mdbiWOinPv+l68p2LPNI3 e4ks5jPyw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jnk4e-0006tS-HM; Tue, 23 Jun 2020 14:38:56 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jnk4c-0006s2-0K for linux-arm-kernel@lists.infradead.org; Tue, 23 Jun 2020 14:38:54 +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 6DD2731B; Tue, 23 Jun 2020 07:38:51 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DC5293F71E; Tue, 23 Jun 2020 07:38:49 -0700 (PDT) Date: Tue, 23 Jun 2020 15:38:47 +0100 From: Dave Martin To: "Eric W. Biederman" Subject: Re: [PATCH v8] arm64: Expose FAR_EL1 tag bits in siginfo Message-ID: <20200623143846.GX25945@arm.com> References: <20200623020134.16655-1-pcc@google.com> <87sgemrlgc.fsf@x220.int.ebiederm.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87sgemrlgc.fsf@x220.int.ebiederm.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Will Deacon , Catalin Marinas , Kevin Brodsky , Oleg Nesterov , Kostya Serebryany , Evgenii Stepanov , Andrey Konovalov , Vincenzo Frascino , Peter Collingbourne , Linux ARM , Richard Henderson 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 Tue, Jun 23, 2020 at 07:54:59AM -0500, Eric W. Biederman wrote: > Peter Collingbourne writes: > > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > > index 47f651df781c..a8380a2b6361 100644 > > --- a/arch/arm64/kernel/traps.c > > +++ b/arch/arm64/kernel/traps.c > > @@ -235,20 +235,41 @@ static void arm64_show_signal(int signo, const char *str) > > } > > > > void arm64_force_sig_fault(int signo, int code, void __user *addr, > > + unsigned long far, unsigned char far_tb_mask, > > const char *str) > > { > > arm64_show_signal(signo, str); > > - if (signo == SIGKILL) > > + if (signo == SIGKILL) { > > force_sig(SIGKILL); > > - else > > - force_sig_fault(signo, code, addr); > > + } else { > > + struct kernel_siginfo info; > > + clear_siginfo(&info); > > + info.si_signo = signo; > > + info.si_errno = 0; > > + info.si_code = code; > > + info.si_addr = addr; > > + info.si_addr_top_byte = (far >> 56) & far_tb_mask; > > + info.si_addr_top_byte_mask = far_tb_mask; > > + force_sig_info(&info); > > + } > > } > > > > void arm64_force_sig_mceerr(int code, void __user *addr, short lsb, > > - const char *str) > > + unsigned long far, const char *str) > > { > > + struct kernel_siginfo info; > > + > > arm64_show_signal(SIGBUS, str); > > - force_sig_mceerr(code, addr, lsb); > > + > > + clear_siginfo(&info); > > + info.si_signo = SIGBUS; > > + info.si_errno = 0; > > + info.si_code = code; > > + info.si_addr = addr; > > + info.si_addr_lsb = lsb; > > + info.si_addr_top_byte = far >> 56; > > + info.si_addr_top_byte_mask = 0xff; > > + force_sig_info(&info); > > } > > I have a real problem with this construction. force_sig_info is not an > interface that should be used for anything except to define a wrapper > that takes it's parameters. Can you elaborate? How would you do this king of thing. AIUI we absolutely need a forced signal here, we need to supply metadata, and we don't have to open-code all that at every relevant signal generation site... > It is not clear to me that if you have adapted siginfo_layout. Garbled sentence? > > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > > index cb3d6c267181..6dd82373eb2d 100644 > > --- a/include/uapi/asm-generic/siginfo.h > > +++ b/include/uapi/asm-generic/siginfo.h > > @@ -91,6 +91,14 @@ union __sifields { > > char _dummy_pkey[__ADDR_BND_PKEY_PAD]; > > __u32 _pkey; > > } _addr_pkey; > > +#ifdef __aarch64__ > > + /* used with all si_codes */ > > + struct { > > + short _dummy_top_byte; ^ What's this for? I don't have Eric's insight here. > > + unsigned char _top_byte; > > + unsigned char _top_byte_mask; > > + } _addr_top_byte; > > +#endif > > }; > > } _sigfault; > > > > Why the _dummy_top_byte? Oh I see it should be spelled "short _addr_lsb;". > > Please remove the "#ifdef __aarch64__". If at all possible we want to > design this so any other architecture who has this challenge can use the > code. The kind of code does not get enough attention/maintenance if it > is built for a single architecture. Does this belong in the user-facing siginfo? It seems a bit strange, when other closely-related information such as esr_context is in the arch-specific signal frame. If trying to make this reusable, I wonder if we should have some sort of "address attributes" field. An alternative approach would be to add some opaque "arch_data" field, that the arch code can go look at when delivering the signal. I think that's all we were trying to achieve here: tack some arch private data onto the signal, to avoid having to stash the same info in thread_info and pray that it doesn't get clobbered in between signal generation and delivery. At signal delivery time, the arch signal delivery code could inspect this data and emit it into the signal frame as appropriate for the arch. [...] Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel