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=-11.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 EF93AC2BC11 for ; Tue, 8 Sep 2020 19:50:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD42020768 for ; Tue, 8 Sep 2020 19:50:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730760AbgIHTuv (ORCPT ); Tue, 8 Sep 2020 15:50:51 -0400 Received: from foss.arm.com ([217.140.110.172]:56346 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730807AbgIHPuk (ORCPT ); Tue, 8 Sep 2020 11:50:40 -0400 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 9B43F16F3; Tue, 8 Sep 2020 08:13: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 BC0D33F73C; Tue, 8 Sep 2020 08:13:49 -0700 (PDT) Date: Tue, 8 Sep 2020 16:13:47 +0100 From: Dave Martin To: Peter Collingbourne Cc: Catalin Marinas , Evgenii Stepanov , Kostya Serebryany , Vincenzo Frascino , Will Deacon , Oleg Nesterov , "Eric W. Biederman" , "James E.J. Bottomley" , linux-parisc@vger.kernel.org, Andrey Konovalov , Kevin Brodsky , David Spickett , Linux ARM , Richard Henderson Subject: Re: [PATCH v10 7/7] arm64: expose FAR_EL1 tag bits in siginfo Message-ID: <20200908151347.GX6642@arm.com> References: <5370542d2ab88a9713ff548359e422337cdac1ee.1598072840.git.pcc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5370542d2ab88a9713ff548359e422337cdac1ee.1598072840.git.pcc@google.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org On Fri, Aug 21, 2020 at 10:10:17PM -0700, Peter Collingbourne wrote: > The kernel currently clears the tag bits (i.e. bits 56-63) in the fault > address exposed via siginfo.si_addr and sigcontext.fault_address. However, > the tag bits may be needed by tools in order to accurately diagnose > memory errors, such as HWASan [1] or future tools based on the Memory > Tagging Extension (MTE). > > We should not stop clearing these bits in the existing fault address > fields, because there may be existing userspace applications that are > expecting the tag bits to be cleared. Instead, create a new pair of > union fields in siginfo._sigfault, and store the tag bits of FAR_EL1 > there, together with a mask specifying which bits are valid. > > A flag is added to si_xflags to allow userspace to determine whether > the values in the fields are valid. > > [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html > > Signed-off-by: Peter Collingbourne > --- > View this change in Gerrit: https://linux-review.googlesource.com/q/Ia8876bad8c798e0a32df7c2ce1256c4771c81446 > > v10: > - rename the flag to SIXFLAG_ADDR_IGNORED_BITS > - use an arch hook to specify which bits are ignored, instead > of passing them explicitly > - while refactoring for the arch hook, noticed that my previous > patches missed a case involving cache maintenance instructions, > so expose the tag bits for that signal as well > > v9: > - make the ignored bits fields generic > - add some new dependent patches that prepare us to store the > field in such a way that userspace can detect their presence > > v8: > - rebase onto 5.8rc2 > > v7: > - switch to a new siginfo field instead of using sigcontext > - merge the patch back into one since the other patches are now > unnecessary > > v6: > - move fault address and fault code into the kernel_siginfo data structure > - split the patch in three since it was getting large and now has > generic and arch-specific parts > > v5: > - add padding to fault_addr_top_byte_context in order to ensure the correct > size and preserve sp alignment > > v4: > - expose only the tag bits in the context instead of the entire FAR_EL1 > - remove mention of the new context from the sigcontext.__reserved[] note > > v3: > - add documentation to tagged-pointers.rst > - update comments in sigcontext.h > > v2: > - revert changes to hw_breakpoint.c > - rename set_thread_esr to set_thread_far_esr > > Documentation/arm64/tagged-pointers.rst | 21 ++++++--- > arch/arm64/include/asm/exception.h | 2 +- > arch/arm64/include/asm/signal.h | 17 +++++++ > arch/arm64/include/asm/system_misc.h | 2 +- > arch/arm64/include/asm/traps.h | 6 +-- > arch/arm64/kernel/debug-monitors.c | 5 +-- > arch/arm64/kernel/entry-common.c | 2 - > arch/arm64/kernel/ptrace.c | 7 +-- > arch/arm64/kernel/sys_compat.c | 5 +-- > arch/arm64/kernel/traps.c | 29 ++++++------ > arch/arm64/mm/fault.c | 59 +++++++++++++------------ > arch/x86/kernel/signal_compat.c | 4 +- > include/linux/compat.h | 2 + > include/linux/signal.h | 8 ++++ > include/uapi/asm-generic/siginfo.h | 10 +++++ > kernel/signal.c | 14 +++++- > 16 files changed, 122 insertions(+), 71 deletions(-) > create mode 100644 arch/arm64/include/asm/signal.h > > diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst > index eab4323609b9..14273160b38b 100644 > --- a/Documentation/arm64/tagged-pointers.rst > +++ b/Documentation/arm64/tagged-pointers.rst > @@ -53,12 +53,21 @@ visibility. > Preserving tags > --------------- > > -Non-zero tags are not preserved when delivering signals. This means that > -signal handlers in applications making use of tags cannot rely on the > -tag information for user virtual addresses being maintained for fields > -inside siginfo_t. One exception to this rule is for signals raised in > -response to watchpoint debug exceptions, where the tag information will > -be preserved. > +Non-zero tags are not preserved in the fault address fields > +siginfo.si_addr or sigcontext.fault_address when delivering > +signals. This means that signal handlers in applications making use > +of tags cannot rely on the tag information for user virtual addresses > +being maintained in these fields. One exception to this rule is for > +signals raised in response to watchpoint debug exceptions, where the > +tag information will be preserved. > + > +The fault address tag is preserved in the si_addr_ignored_bits field > +of siginfo, which is set for signals raised in response to data aborts > +and instruction aborts. The si_addr_ignored_bits_mask field indicates > +which bits of the field are valid. The validity of these fields is > +indicated by the SIXFLAG_ADDR_IGNORED_BITS flag in siginfo.si_xflags, > +and the validity of si_xflags in turn is indicated by the kernel > +indicating support for the sigaction.sa_flags flag SA_XFLAGS. > > The architecture prevents the use of a tagged PC, so the upper byte will > be set to a sign-extension of bit 55 on exception return. > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h > index 7577a754d443..950d55dae948 100644 > --- a/arch/arm64/include/asm/exception.h > +++ b/arch/arm64/include/asm/exception.h > @@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr) > } > > asmlinkage void enter_from_user_mode(void); > -void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs); > +void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs); > void do_undefinstr(struct pt_regs *regs); > void do_bti(struct pt_regs *regs); > asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr); [...] > diff --git a/include/linux/compat.h b/include/linux/compat.h > index 55d4228dfd88..273146cf30fd 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -234,6 +234,8 @@ typedef struct compat_siginfo { > compat_uptr_t _pad[6]; > }; > compat_uptr_t _xflags; > + compat_uptr_t _addr_ignored_bits; > + compat_uptr_t _addr_ignored_bits_mask; > } _sigfault; > > /* SIGPOLL */ > diff --git a/include/linux/signal.h b/include/linux/signal.h > index 3edbf54493ee..b4c473c12a9b 100644 > --- a/include/linux/signal.h > +++ b/include/linux/signal.h > @@ -480,4 +480,12 @@ struct seq_file; > extern void render_sigset_t(struct seq_file *, const char *, sigset_t *); > #endif Can we have a comment here explaining what this helper needs to do, if defined? > +#ifndef arch_addr_ignored_bits_mask > +static inline unsigned long arch_addr_ignored_bits_mask(unsigned long sig, > + unsigned long si_code) > +{ > + return 0; > +} > +#endif > + > #endif /* _LINUX_SIGNAL_H */ > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 1fbd88d64f38..e314a38ce2d0 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -94,6 +94,8 @@ union __sifields { > void *_pad[6]; > }; > unsigned long _xflags; > + unsigned long _addr_ignored_bits; > + unsigned long _addr_ignored_bits_mask; > } _sigfault; > > /* SIGPOLL */ > @@ -156,6 +158,8 @@ typedef struct siginfo { > #define si_addr_lsb _sifields._sigfault._addr_lsb > /* si_xflags is only valid if 0 <= si_code < SI_KERNEL */ > #define si_xflags _sifields._sigfault._xflags > +#define si_addr_ignored_bits _sifields._sigfault._addr_ignored_bits > +#define si_addr_ignored_bits_mask _sifields._sigfault._addr_ignored_bits_mask > #define si_lower _sifields._sigfault._addr_bnd._lower > #define si_upper _sifields._sigfault._addr_bnd._upper > #define si_pkey _sifields._sigfault._addr_pkey._pkey > @@ -296,6 +300,12 @@ typedef struct siginfo { > #define EMT_TAGOVF 1 /* tag overflow */ > #define NSIGEMT 1 > > +/* > + * SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT si_xflags > + */ > +#define SIXFLAG_ADDR_IGNORED_BITS 1 > +/* si_addr_ignored_bits{,_mask} fields valid */ > + I'm still uneasy about the "ignored bits" nomenclature, because the bits aren't ignored, and because at the C language level above, they generally _are_ considered part of the address. I don't have a great suggestion for a new name, though. If we just consider the si_addr_ignored_bits to be attributes that accompany the address, it might make sense to call it si_addr_attr_bits_mask or similar. That may or may not be considered to be less confusing. > /* > * sigevent definitions > * > diff --git a/kernel/signal.c b/kernel/signal.c > index 4259903b95cb..29654652d3aa 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1652,11 +1652,17 @@ void force_sigsegv(int sig) > static void set_sigfault_common_fields(struct kernel_siginfo *info, int sig, > int code, void __user *addr) > { > + unsigned long addr_long = (unsigned long)addr; > + unsigned long ignored_bits_mask = > + arch_addr_ignored_bits_mask(sig, code); > + > info->si_signo = sig; > info->si_errno = 0; > info->si_code = code; > - info->si_addr = addr; > - info->si_xflags = 0; > + info->si_addr = (void __user *)(addr_long & ~ignored_bits_mask); > + info->si_xflags = SIXFLAG_ADDR_IGNORED_BITS; > + info->si_addr_ignored_bits = addr_long & ignored_bits_mask; > + info->si_addr_ignored_bits_mask = ignored_bits_mask; Could we report the ignored bits optionally? i.e., if arch_addr_ignored_bits_mask() == 0, then we could perhaps leave SIXFLAG_ADDR_IGNORED_BITS clear in si_xflags, and just set si_addr_ignored_bits{,_mask} to zeros. I can't decide myself whether this would be a good idea or not... > } > > int force_sig_fault_to_task(int sig, int code, void __user *addr > @@ -3271,6 +3277,8 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > to->si_trapno = from->si_trapno; > #endif > to->si_xflags = from->si_xflags; > + to->si_addr_ignored_bits = from->si_addr_ignored_bits; > + to->si_addr_ignored_bits_mask = from->si_addr_ignored_bits_mask; What happens if we're delivering a signal to a compat process? It looks like we apply all the usual logic, but si_addr_ignored_bits and si_addr_ignored_bits_mask simply get truncated. That might be reasonable -- we don't care about bits that don't exist for compat, irrespective of what they mean for native -- but it's probably worth a comment. > } > > switch (layout) { > @@ -3347,6 +3355,8 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > to->si_trapno = from->si_trapno; > #endif > to->si_xflags = from->si_xflags; > + to->si_addr_ignored_bits = from->si_addr_ignored_bits; > + to->si_addr_ignored_bits_mask = from->si_addr_ignored_bits_mask; Otherwise, the patch looks reasonable. Cheers ---Dave 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=-11.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,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 DDB44C433E2 for ; Tue, 8 Sep 2020 15:15:27 +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 5AC5823D3B for ; Tue, 8 Sep 2020 15:15:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="WuSy225Z" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5AC5823D3B 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=4afEvz1ZJXfS2uwHCG49+ZXwLNbGhJHweAkzquGUvqw=; b=WuSy225Zg1P4RpoQzOoWam6QZ bndjZ+wBsdRcGm6qWx74uiUy/VG1JcMZlcoQQRk8H39NUILgO2VbvY+vyK1YaQtRxNr+FdcVbZSq/ RhFwsrlvThWIQXTwuk8vATVaTHVQP+WWvjmLAkdP3lTunYn3GEWL2nwiso+uIK+s/sI1FyHTHtkbu YJvmbG3JDgssyZ71PeHy8Sio0qUQzKl/P/5c4DFDhptD6lV1UTF2b1xdD6aKwNuudh4jIumoygl2b fryOGdzZkahNONGfDgO8etHDd13o2rP+s4C42h95mrtlnnqWBkLtjRgnpvwKhaIBHmBXfujfV8qP9 h4mYkNxEg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFfJp-0007zA-VM; Tue, 08 Sep 2020 15:14:02 +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 1kFfJg-0007vj-LM for linux-arm-kernel@lists.infradead.org; Tue, 08 Sep 2020 15:13:55 +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 9B43F16F3; Tue, 8 Sep 2020 08:13: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 BC0D33F73C; Tue, 8 Sep 2020 08:13:49 -0700 (PDT) Date: Tue, 8 Sep 2020 16:13:47 +0100 From: Dave Martin To: Peter Collingbourne Subject: Re: [PATCH v10 7/7] arm64: expose FAR_EL1 tag bits in siginfo Message-ID: <20200908151347.GX6642@arm.com> References: <5370542d2ab88a9713ff548359e422337cdac1ee.1598072840.git.pcc@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5370542d2ab88a9713ff548359e422337cdac1ee.1598072840.git.pcc@google.com> 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-20200908_111352_929381_59DAA160 X-CRM114-Status: GOOD ( 48.39 ) 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 , linux-parisc@vger.kernel.org, Catalin Marinas , Kevin Brodsky , Oleg Nesterov , "James E.J. Bottomley" , Kostya Serebryany , "Eric W. Biederman" , Andrey Konovalov , David Spickett , 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 Fri, Aug 21, 2020 at 10:10:17PM -0700, Peter Collingbourne wrote: > The kernel currently clears the tag bits (i.e. bits 56-63) in the fault > address exposed via siginfo.si_addr and sigcontext.fault_address. However, > the tag bits may be needed by tools in order to accurately diagnose > memory errors, such as HWASan [1] or future tools based on the Memory > Tagging Extension (MTE). > > We should not stop clearing these bits in the existing fault address > fields, because there may be existing userspace applications that are > expecting the tag bits to be cleared. Instead, create a new pair of > union fields in siginfo._sigfault, and store the tag bits of FAR_EL1 > there, together with a mask specifying which bits are valid. > > A flag is added to si_xflags to allow userspace to determine whether > the values in the fields are valid. > > [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html > > Signed-off-by: Peter Collingbourne > --- > View this change in Gerrit: https://linux-review.googlesource.com/q/Ia8876bad8c798e0a32df7c2ce1256c4771c81446 > > v10: > - rename the flag to SIXFLAG_ADDR_IGNORED_BITS > - use an arch hook to specify which bits are ignored, instead > of passing them explicitly > - while refactoring for the arch hook, noticed that my previous > patches missed a case involving cache maintenance instructions, > so expose the tag bits for that signal as well > > v9: > - make the ignored bits fields generic > - add some new dependent patches that prepare us to store the > field in such a way that userspace can detect their presence > > v8: > - rebase onto 5.8rc2 > > v7: > - switch to a new siginfo field instead of using sigcontext > - merge the patch back into one since the other patches are now > unnecessary > > v6: > - move fault address and fault code into the kernel_siginfo data structure > - split the patch in three since it was getting large and now has > generic and arch-specific parts > > v5: > - add padding to fault_addr_top_byte_context in order to ensure the correct > size and preserve sp alignment > > v4: > - expose only the tag bits in the context instead of the entire FAR_EL1 > - remove mention of the new context from the sigcontext.__reserved[] note > > v3: > - add documentation to tagged-pointers.rst > - update comments in sigcontext.h > > v2: > - revert changes to hw_breakpoint.c > - rename set_thread_esr to set_thread_far_esr > > Documentation/arm64/tagged-pointers.rst | 21 ++++++--- > arch/arm64/include/asm/exception.h | 2 +- > arch/arm64/include/asm/signal.h | 17 +++++++ > arch/arm64/include/asm/system_misc.h | 2 +- > arch/arm64/include/asm/traps.h | 6 +-- > arch/arm64/kernel/debug-monitors.c | 5 +-- > arch/arm64/kernel/entry-common.c | 2 - > arch/arm64/kernel/ptrace.c | 7 +-- > arch/arm64/kernel/sys_compat.c | 5 +-- > arch/arm64/kernel/traps.c | 29 ++++++------ > arch/arm64/mm/fault.c | 59 +++++++++++++------------ > arch/x86/kernel/signal_compat.c | 4 +- > include/linux/compat.h | 2 + > include/linux/signal.h | 8 ++++ > include/uapi/asm-generic/siginfo.h | 10 +++++ > kernel/signal.c | 14 +++++- > 16 files changed, 122 insertions(+), 71 deletions(-) > create mode 100644 arch/arm64/include/asm/signal.h > > diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst > index eab4323609b9..14273160b38b 100644 > --- a/Documentation/arm64/tagged-pointers.rst > +++ b/Documentation/arm64/tagged-pointers.rst > @@ -53,12 +53,21 @@ visibility. > Preserving tags > --------------- > > -Non-zero tags are not preserved when delivering signals. This means that > -signal handlers in applications making use of tags cannot rely on the > -tag information for user virtual addresses being maintained for fields > -inside siginfo_t. One exception to this rule is for signals raised in > -response to watchpoint debug exceptions, where the tag information will > -be preserved. > +Non-zero tags are not preserved in the fault address fields > +siginfo.si_addr or sigcontext.fault_address when delivering > +signals. This means that signal handlers in applications making use > +of tags cannot rely on the tag information for user virtual addresses > +being maintained in these fields. One exception to this rule is for > +signals raised in response to watchpoint debug exceptions, where the > +tag information will be preserved. > + > +The fault address tag is preserved in the si_addr_ignored_bits field > +of siginfo, which is set for signals raised in response to data aborts > +and instruction aborts. The si_addr_ignored_bits_mask field indicates > +which bits of the field are valid. The validity of these fields is > +indicated by the SIXFLAG_ADDR_IGNORED_BITS flag in siginfo.si_xflags, > +and the validity of si_xflags in turn is indicated by the kernel > +indicating support for the sigaction.sa_flags flag SA_XFLAGS. > > The architecture prevents the use of a tagged PC, so the upper byte will > be set to a sign-extension of bit 55 on exception return. > diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h > index 7577a754d443..950d55dae948 100644 > --- a/arch/arm64/include/asm/exception.h > +++ b/arch/arm64/include/asm/exception.h > @@ -32,7 +32,7 @@ static inline u32 disr_to_esr(u64 disr) > } > > asmlinkage void enter_from_user_mode(void); > -void do_mem_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs); > +void do_mem_abort(unsigned long far, unsigned int esr, struct pt_regs *regs); > void do_undefinstr(struct pt_regs *regs); > void do_bti(struct pt_regs *regs); > asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr); [...] > diff --git a/include/linux/compat.h b/include/linux/compat.h > index 55d4228dfd88..273146cf30fd 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -234,6 +234,8 @@ typedef struct compat_siginfo { > compat_uptr_t _pad[6]; > }; > compat_uptr_t _xflags; > + compat_uptr_t _addr_ignored_bits; > + compat_uptr_t _addr_ignored_bits_mask; > } _sigfault; > > /* SIGPOLL */ > diff --git a/include/linux/signal.h b/include/linux/signal.h > index 3edbf54493ee..b4c473c12a9b 100644 > --- a/include/linux/signal.h > +++ b/include/linux/signal.h > @@ -480,4 +480,12 @@ struct seq_file; > extern void render_sigset_t(struct seq_file *, const char *, sigset_t *); > #endif Can we have a comment here explaining what this helper needs to do, if defined? > +#ifndef arch_addr_ignored_bits_mask > +static inline unsigned long arch_addr_ignored_bits_mask(unsigned long sig, > + unsigned long si_code) > +{ > + return 0; > +} > +#endif > + > #endif /* _LINUX_SIGNAL_H */ > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 1fbd88d64f38..e314a38ce2d0 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -94,6 +94,8 @@ union __sifields { > void *_pad[6]; > }; > unsigned long _xflags; > + unsigned long _addr_ignored_bits; > + unsigned long _addr_ignored_bits_mask; > } _sigfault; > > /* SIGPOLL */ > @@ -156,6 +158,8 @@ typedef struct siginfo { > #define si_addr_lsb _sifields._sigfault._addr_lsb > /* si_xflags is only valid if 0 <= si_code < SI_KERNEL */ > #define si_xflags _sifields._sigfault._xflags > +#define si_addr_ignored_bits _sifields._sigfault._addr_ignored_bits > +#define si_addr_ignored_bits_mask _sifields._sigfault._addr_ignored_bits_mask > #define si_lower _sifields._sigfault._addr_bnd._lower > #define si_upper _sifields._sigfault._addr_bnd._upper > #define si_pkey _sifields._sigfault._addr_pkey._pkey > @@ -296,6 +300,12 @@ typedef struct siginfo { > #define EMT_TAGOVF 1 /* tag overflow */ > #define NSIGEMT 1 > > +/* > + * SIGILL, SIGFPE, SIGSEGV, SIGBUS, SIGTRAP, SIGEMT si_xflags > + */ > +#define SIXFLAG_ADDR_IGNORED_BITS 1 > +/* si_addr_ignored_bits{,_mask} fields valid */ > + I'm still uneasy about the "ignored bits" nomenclature, because the bits aren't ignored, and because at the C language level above, they generally _are_ considered part of the address. I don't have a great suggestion for a new name, though. If we just consider the si_addr_ignored_bits to be attributes that accompany the address, it might make sense to call it si_addr_attr_bits_mask or similar. That may or may not be considered to be less confusing. > /* > * sigevent definitions > * > diff --git a/kernel/signal.c b/kernel/signal.c > index 4259903b95cb..29654652d3aa 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1652,11 +1652,17 @@ void force_sigsegv(int sig) > static void set_sigfault_common_fields(struct kernel_siginfo *info, int sig, > int code, void __user *addr) > { > + unsigned long addr_long = (unsigned long)addr; > + unsigned long ignored_bits_mask = > + arch_addr_ignored_bits_mask(sig, code); > + > info->si_signo = sig; > info->si_errno = 0; > info->si_code = code; > - info->si_addr = addr; > - info->si_xflags = 0; > + info->si_addr = (void __user *)(addr_long & ~ignored_bits_mask); > + info->si_xflags = SIXFLAG_ADDR_IGNORED_BITS; > + info->si_addr_ignored_bits = addr_long & ignored_bits_mask; > + info->si_addr_ignored_bits_mask = ignored_bits_mask; Could we report the ignored bits optionally? i.e., if arch_addr_ignored_bits_mask() == 0, then we could perhaps leave SIXFLAG_ADDR_IGNORED_BITS clear in si_xflags, and just set si_addr_ignored_bits{,_mask} to zeros. I can't decide myself whether this would be a good idea or not... > } > > int force_sig_fault_to_task(int sig, int code, void __user *addr > @@ -3271,6 +3277,8 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > to->si_trapno = from->si_trapno; > #endif > to->si_xflags = from->si_xflags; > + to->si_addr_ignored_bits = from->si_addr_ignored_bits; > + to->si_addr_ignored_bits_mask = from->si_addr_ignored_bits_mask; What happens if we're delivering a signal to a compat process? It looks like we apply all the usual logic, but si_addr_ignored_bits and si_addr_ignored_bits_mask simply get truncated. That might be reasonable -- we don't care about bits that don't exist for compat, irrespective of what they mean for native -- but it's probably worth a comment. > } > > switch (layout) { > @@ -3347,6 +3355,8 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > to->si_trapno = from->si_trapno; > #endif > to->si_xflags = from->si_xflags; > + to->si_addr_ignored_bits = from->si_addr_ignored_bits; > + to->si_addr_ignored_bits_mask = from->si_addr_ignored_bits_mask; Otherwise, the patch looks reasonable. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel