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.3 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,URIBL_BLOCKED,USER_AGENT_SANE_1 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 59FFDC433E1 for ; Tue, 18 Aug 2020 13:51:58 +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 232F3206B5 for ; Tue, 18 Aug 2020 13:51:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="rim3SOa+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 232F3206B5 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=e9Y+hYQmfVF3bLdvBpu5x5QPkwOC4zTCBJcmRwNRMg4=; b=rim3SOa+B901AC8Zx8yMn0Bvc YhxuZsD6bPfWTJrqAE3ZZVneSa9GCP7C4UP11m4Dbagp5QnPSuvL5We/wrx5rUqZ/IBaWcrohxHkX VA04wOnmn9jM3MgS+Yp5zsL1hzab7UGUdnKeDTP57kKdIcLZfFOliVYTl6W6SIN6faPoPeWLTxlNt ETyGXpY277FQyKbqtEdmQyXUulpTK+pas5rQBouVXW2akMQ+Nfufa15eKd+QGROPnkO1XjVS0byPH X+OBD9xxBoNQhYaXf73BLEd3VekQ8CNHIYVbHm4FjJb6HsLdO9/mGbmIg+kxWVJQU5HlAkLMmbwcq PHTx5KHAA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k820P-000728-34; Tue, 18 Aug 2020 13:50:25 +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 1k820M-00071W-Qp for linux-arm-kernel@lists.infradead.org; Tue, 18 Aug 2020 13:50:23 +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 C9DD91FB; Tue, 18 Aug 2020 06:50:16 -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 38E1C3F66B; Tue, 18 Aug 2020 06:50:15 -0700 (PDT) Date: Tue, 18 Aug 2020 14:50:13 +0100 From: Dave Martin To: Peter Collingbourne Subject: Re: [PATCH v8] arm64: Expose FAR_EL1 tag bits in siginfo Message-ID: <20200818135010.GB6642@arm.com> References: <20200707141945.GK10992@arm.com> <20200708110019.GM10992@arm.com> <20200708135808.GN10992@arm.com> <20200713132407.GQ10992@arm.com> <20200714173625.GB30452@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200818_095022_971896_C5ED2D79 X-CRM114-Status: GOOD ( 47.00 ) 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: Linux ARM , Catalin Marinas , Kevin Brodsky , Oleg Nesterov , Kostya Serebryany , "Eric W. Biederman" , Andrey Konovalov , Vincenzo Frascino , Will Deacon , Evgenii Stepanov , 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 Mon, Aug 17, 2020 at 08:16:42PM -0700, Peter Collingbourne wrote: > On Tue, Jul 14, 2020 at 10:36 AM Dave Martin wrote: [...] > > As a final random idea to add to the mix, we could add two or more > > flags in sa_flags, and require the kernel to transform them in a > > specific way, say: > > > > #define SA_WANT_FLAGS 0x00c700000 > > #define SA_HAVE_FLAGS 0x009200000 > > #define SA_FLAGS_MASK 0x00ff00000 > > > > volatile sig_atomic_t have_flags = 0; > > > > sa.sa_flags |= SA_WANT_FLAGS; > > if (sigaction(n, &sa, NULL)) > > if (!sigaction(n, NULL, &sa) && > > (sa.sa_flags & SA_FLAGS_MASK) == SA_HAVE_FLAGS) > > have_flags = 1; > > > > This is at least proof against "dumb readback". > > > > Provided that the handler can cope with the have_flags == 0 case and > > just reads the flag once per call, I don't think we would need to worry > > about races. > > > > Of course, an interceptor that doesn't understand this mechanism and > > munges or manufactures its own siginfo might still fail to properly > > initialise our new field before passing it on to a signal handler that > > is expecting it. But that's already broken: such an interceptor might > > also not understand new si_codes that the client code absolutely relies > > on. And new si_codes _do_ get added (that's another extensibility fail > > in the existing signal API). > > > > > > So... overall, maybe a bit in the signal number isn't a lot worse, and > > perhaps it _will_ lead to cleaner failures. > > > > Really, I don't see a way to solve it properly without a new API. > > I started on implementing my signal number bit idea, and in the > process of doing so came up with another idea that may be better from > the "don't abuse existing arguments" perspective. It involves a > sigaction protocol similar to the one that you describe above, but it > only requires one new bit (plus one bit per new flag) so it is less > wasteful of sa_flags bits. > > The idea is twofold: > > 1. Require the kernel to clear unknown flag bits in sa_flags when > passing them back in oldact. I suppose that this is technically a > behavior change for sigaction, but critically, this change in behavior > only applies to unallocated flags, which we are free to change the > meaning of. We can simply define each existing unallocated flag bit to > mean "clear this bit in oldact (unless the bit becomes supported in > the future)". There was already code doing something similar in a > limited fashion on x86, which we can remove by using this approach. Sounds reasonable. It's quite hard to imagine how software could accidentally rely on unallocated sa_flags bits reading back out unmodified through sigaction(). Software that deliberately relies on this for some bit that is never allocated would be obviously non POSIX compliant. > 2. Define a flag bit SA_UNSUPPORTED which will never be supported by > the kernel. Now userspace can use the fact that the bit has been > cleared to mean that it can trust that other unsupported bits have > also been cleared. > > Now we may have code like this: > > #define SA_UNSUPPORTED 0x400 > #define SA_XFLAGS 0x800 > > volatile sig_atomic_t have_xflags = 0; > > sa.sa_flags |= SA_UNSUPPORTED | SA_XFLAGS; > if (sigaction(n, &sa, NULL)) > if (!sigaction(n, NULL, &sa) && > !(sa.sa_flags & SA_UNSUPPORTED) && > (sa.sa_flags & SA_XFLAGS)) > have_xflags = 1; OK, so I think the novelty here is that we detect support by requiring the kernel to clear one bit while preserving another. That does indeed seem more robust: an old kernel (unless bizarrely buggy) would either clear all unsupported bits or preserve them all. Other OSes that extend their sigaction() in line with POSIX would also be highly unlikely to exhibit this behaviour by accident IMHO, making it easier for this to coexist with other people's extensions. Interceptors still may not transparently work with this approach, but I think that's a reasonable price to pay. > > > In the meantime, can I suggest: > > > > (1) Come up with an extensible way of encoding supplementary > > information in siginfo. If the consensus is that zeroing unused > > fields is sufficient and that the kernel and compiler will > > reliably do it, then great. Otherwise, we might need explicit > > flags fields or something. > > I thought about this for a while and concluded that we probably want a > flags field anyway. si_addr_ignored_bits is something of a special > case in the sense that we can define the zero value to mean > "unknown" by taking advantage of the mask field (which I suppose is > something of a flags field), but we can't necessarily say that the > same is true for any fields that we may add in the future. For > example, if we wanted to communicate whether the failing access is a > read or a write, we would need a tristate: read, write and "unknown" > (and arrange for old kernels' behavior to be interpreted as > "unknown"). If we rely on zeroing then we may implement this by adding > a field like: > > char si_access_type; // 0 = unknown, 1 = read, 2 = write > > But that's really just a (slightly wasteful, because we use the entire > byte) flags field, so we may as well define an actual flags field to > begin with and let people add their flags there. > > Unfortunately we can't name it sa_flags because ia64 got there first. > We may consider making the ia64 field generic though (ia64 only uses > one bit of their field, so we would have 31 free bits). In the > meantime, I added a separate field, sa_xflags. Seems fair enough to me. We still have the option to use zeroing for detection of fields where it works. Would SA_XFLAGS imply zeroing of unallocated siginfo fields? This may just be a matter of documentation, if the kernel already does the zeroing today. > > > (2) Hack up any simple mechanism (such as your signal number flag) for > > requesting/detecting the extra information. > > > > Along with an illustration of a application of the mechanism (i.e., > > reporting address tag bits), this should at least provide a basis for > > further review. > > > > We can then try to swap in a different mechanism for (2) if people have > > still have concerns (or it not, keep it). > > Sounds good. Apologies for not replying sooner, I was hoping that Eric > would chime in so that I would get a sense of which approach he would > prefer (so that I wouldn't spend as much time implementing in an > undesired direction), then this fell off my radar. I decided to go > with the SA_UNSUPPORTED approach that I mentioned above for now, and > I'll send a v9 with that implemented shortly. Most of the change is > about letting the architecture-independent code know which bits are > supported, so it should be easy to replace the detection mechanism > with another idea like the signal number bit. No worries, I think various people have had distractions (I certainly have ... but I digress). I'll take a look at your v9. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel