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,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=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 AF7DBC433E5 for ; Wed, 19 Aug 2020 15:41: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 6FFF920639 for ; Wed, 19 Aug 2020 15:41:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="CxDUXaSd" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6FFF920639 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=fL0jq5g77h1VudQJPDjrAFs8Nqhj8Ag+FJTxJZEr4Wo=; b=CxDUXaSdv13QLtjx7lUddWjaI mWOusgEQjVKClNAPrYp3VRDkvzGs8R06RmTx5O7O6GehuFIWl1v8SecrEgik296ojviATPQ6DiZvK bhcqKO2X52Xb5gfoCmTM2jJ5SZlMzWs2mhTCiLF3+pDdFZo2krcf64mJ1VQB2Gk37qKOkvGs6rgwh IbwWlzcwxtnohyIbBNGlY6MKDGVe0kapZMAtf4/VwsYOFvBr/Rl+SLj2FpHntw5TEarKZrAYesF2d jZ47G1rgcqZ06YXUw1/Cpsg1cVGsEp/y3enYbgX0WkMdlgzjLvW1vquJbe5W4a0QSuV0JXyJrJA1/ qg4BQeRMQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8QCc-0007u8-NN; Wed, 19 Aug 2020 15:40:38 +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 1k8QCZ-0007tW-4u for linux-arm-kernel@lists.infradead.org; Wed, 19 Aug 2020 15:40:36 +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 4E08B1045; Wed, 19 Aug 2020 08:40:33 -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 6C6BF3F71F; Wed, 19 Aug 2020 08:40:31 -0700 (PDT) Date: Wed, 19 Aug 2020 16:40:29 +0100 From: Dave Martin To: Peter Collingbourne Subject: Re: [PATCH v9 5/6] signal: define the field siginfo.si_xflags Message-ID: <20200819154028.GH6642@arm.com> References: 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-20200819_114035_324549_7E155243 X-CRM114-Status: GOOD ( 37.05 ) 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 Mon, Aug 17, 2020 at 08:33:50PM -0700, Peter Collingbourne wrote: > This field will contain flags that may be used by signal handlers to > determine whether other fields in the _sigfault portion of siginfo are > valid. An example use case is the following patch, which introduces > the si_addr_ignored_bits{,_mask} fields. > > A new sigcontext flag, SA_XFLAGS, is introduced in order to allow > a signal handler to require the kernel to set the field (but note > that the field will be set anyway if the kernel supports the flag, > regardless of its value). In combination with the previous patches, > this allows a userspace program to determine whether the kernel will > set the field. > > Ideally this field could have just been named si_flags, but that > name was already taken by ia64, so a different name was chosen. > > Alternatively, we may consider making ia64's si_flags a generic field > and having it appear at the end of _sigfault (in the same place as > this patch has si_xflags) on non-ia64, keeping it in the same place > on ia64. ia64's si_flags is a 32-bit field with only one flag bit > allocated, so we would have 31 bits to use if we do this. For clarity, is the new si_xflags field supposed to be valid for all signal types, or just certain signals and si_codes? What happens for things like a rt_sigqueueinfo() from userspace? > > Signed-off-by: Peter Collingbourne > --- > View this change in Gerrit: https://linux-review.googlesource.com/q/Ide155ce29366c3eab2a944ae4c51205982e5b8b2 > > arch/arm/include/asm/signal.h | 3 ++- > arch/parisc/include/asm/signal.h | 2 +- > arch/powerpc/platforms/powernv/vas-fault.c | 1 + > include/linux/compat.h | 2 ++ > include/linux/signal_types.h | 4 ++-- > include/uapi/asm-generic/siginfo.h | 3 +++ > include/uapi/asm-generic/signal-defs.h | 4 ++++ > kernel/signal.c | 15 +++++++++++++++ > 8 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/include/asm/signal.h b/arch/arm/include/asm/signal.h > index d1070a783993..6b2630dfe1df 100644 > --- a/arch/arm/include/asm/signal.h > +++ b/arch/arm/include/asm/signal.h > @@ -19,7 +19,8 @@ typedef struct { > > #define SA_UAPI_FLAGS \ > (SA_NOCLDSTOP | SA_NOCLDWAIT | SA_SIGINFO | SA_THIRTYTWO | \ > - SA_RESTORER | SA_ONSTACK | SA_RESTART | SA_NODEFER | SA_RESETHAND) > + SA_RESTORER | SA_ONSTACK | SA_RESTART | SA_NODEFER | SA_RESETHAND | \ > + SA_XFLAGS) > > #define __ARCH_HAS_SA_RESTORER > > diff --git a/arch/parisc/include/asm/signal.h b/arch/parisc/include/asm/signal.h > index ad06e14f6e8a..3582bce44520 100644 > --- a/arch/parisc/include/asm/signal.h > +++ b/arch/parisc/include/asm/signal.h > @@ -23,7 +23,7 @@ typedef struct { > > #define SA_UAPI_FLAGS \ > (SA_ONSTACK | SA_RESETHAND | SA_NOCLDSTOP | SA_SIGINFO | SA_NODEFER | \ > - SA_RESTART | SA_NOCLDWAIT | _SA_SIGGFAULT) > + SA_RESTART | SA_NOCLDWAIT | _SA_SIGGFAULT | SA_XFLAGS) > > #include > > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c > index 3d21fce254b7..3bbb335561f5 100644 > --- a/arch/powerpc/platforms/powernv/vas-fault.c > +++ b/arch/powerpc/platforms/powernv/vas-fault.c > @@ -154,6 +154,7 @@ static void update_csb(struct vas_window *window, > info.si_errno = EFAULT; > info.si_code = SEGV_MAPERR; > info.si_addr = csb_addr; > + info.si_xflags = 0; > > /* > * process will be polling on csb.flags after request is sent to > diff --git a/include/linux/compat.h b/include/linux/compat.h > index d38c4d7e83bd..55d4228dfd88 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -231,7 +231,9 @@ typedef struct compat_siginfo { > char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD]; > u32 _pkey; > } _addr_pkey; > + compat_uptr_t _pad[6]; > }; > + compat_uptr_t _xflags; > } _sigfault; > > /* SIGPOLL */ > diff --git a/include/linux/signal_types.h b/include/linux/signal_types.h > index e792f29b5846..cd3d08dde47a 100644 > --- a/include/linux/signal_types.h > +++ b/include/linux/signal_types.h > @@ -72,11 +72,11 @@ struct ksignal { > #ifdef SA_RESTORER > #define SA_UAPI_FLAGS \ > (SA_NOCLDSTOP | SA_NOCLDWAIT | SA_SIGINFO | SA_ONSTACK | SA_RESTART | \ > - SA_NODEFER | SA_RESETHAND | SA_RESTORER) > + SA_NODEFER | SA_RESETHAND | SA_RESTORER | SA_XFLAGS) > #else > #define SA_UAPI_FLAGS \ > (SA_NOCLDSTOP | SA_NOCLDWAIT | SA_SIGINFO | SA_ONSTACK | SA_RESTART | \ > - SA_NODEFER | SA_RESETHAND) > + SA_NODEFER | SA_RESETHAND | SA_XFLAGS) > #endif > #endif > > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index cb3d6c267181..413d804623c0 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -91,7 +91,9 @@ union __sifields { > char _dummy_pkey[__ADDR_BND_PKEY_PAD]; > __u32 _pkey; > } _addr_pkey; > + void *_pad[6]; > }; > + unsigned long _xflags; > } _sigfault; > > /* SIGPOLL */ > @@ -152,6 +154,7 @@ typedef struct siginfo { > #define si_trapno _sifields._sigfault._trapno > #endif > #define si_addr_lsb _sifields._sigfault._addr_lsb > +#define si_xflags _sifields._sigfault._xflags > #define si_lower _sifields._sigfault._addr_bnd._lower > #define si_upper _sifields._sigfault._addr_bnd._upper > #define si_pkey _sifields._sigfault._addr_pkey._pkey > diff --git a/include/uapi/asm-generic/signal-defs.h b/include/uapi/asm-generic/signal-defs.h > index c30a9c1a77b2..aeee6bb0763b 100644 > --- a/include/uapi/asm-generic/signal-defs.h > +++ b/include/uapi/asm-generic/signal-defs.h > @@ -19,6 +19,9 @@ > * so this bit allows flag bit support to be detected from userspace while > * allowing an old kernel to be distinguished from a kernel that supports every > * flag bit. > + * SA_XFLAGS indicates that the signal handler requires the siginfo.si_xflags > + * field to be valid. Note that if the kernel supports SA_XFLAGS, the field will > + * be valid regardless of the value of this flag. > * > * SA_ONESHOT and SA_NOMASK are the historical Linux names for the Single > * Unix names RESETHAND and NODEFER respectively. > @@ -49,6 +52,7 @@ > * should be avoided for new generic flags: 3, 4, 5, 6, 7, 8, 9, 16, 24, 25, 26. > */ > #define SA_UNSUPPORTED 0x00000400 > +#define SA_XFLAGS 0x00000800 > > #define SA_NOMASK SA_NODEFER > #define SA_ONESHOT SA_RESETHAND > diff --git a/kernel/signal.c b/kernel/signal.c > index 664a6c31137e..72182eed1b8d 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1669,6 +1669,7 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr > info.si_flags = flags; > info.si_isr = isr; > #endif > + info.si_xflags = 0; > return force_sig_info_to_task(&info, t); > } > > @@ -1701,6 +1702,7 @@ int send_sig_fault(int sig, int code, void __user *addr > info.si_flags = flags; > info.si_isr = isr; > #endif > + info.si_xflags = 0; > return send_sig_info(info.si_signo, &info, t); > } > > @@ -1715,6 +1717,7 @@ int force_sig_mceerr(int code, void __user *addr, short lsb) > info.si_code = code; > info.si_addr = addr; > info.si_addr_lsb = lsb; > + info.si_xflags = 0; > return force_sig_info(&info); > } > > @@ -1729,6 +1732,7 @@ int send_sig_mceerr(int code, void __user *addr, short lsb, struct task_struct * > info.si_code = code; > info.si_addr = addr; > info.si_addr_lsb = lsb; > + info.si_xflags = 0; > return send_sig_info(info.si_signo, &info, t); > } > EXPORT_SYMBOL(send_sig_mceerr); > @@ -1744,6 +1748,7 @@ int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper) > info.si_addr = addr; > info.si_lower = lower; > info.si_upper = upper; > + info.si_xflags = 0; > return force_sig_info(&info); > } > > @@ -1758,6 +1763,7 @@ int force_sig_pkuerr(void __user *addr, u32 pkey) > info.si_code = SEGV_PKUERR; > info.si_addr = addr; > info.si_pkey = pkey; > + info.si_xflags = 0; > return force_sig_info(&info); > } > #endif > @@ -1774,6 +1780,7 @@ int force_sig_ptrace_errno_trap(int errno, void __user *addr) > info.si_errno = errno; > info.si_code = TRAP_HWBKPT; > info.si_addr = addr; > + info.si_xflags = 0; > return force_sig_info(&info); > } > > @@ -3290,6 +3297,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > #ifdef __ARCH_SI_TRAPNO > to->si_trapno = from->si_trapno; > #endif > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_MCEERR: > to->si_addr = ptr_to_compat(from->si_addr); > @@ -3297,6 +3305,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > to->si_trapno = from->si_trapno; > #endif > to->si_addr_lsb = from->si_addr_lsb; > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_BNDERR: > to->si_addr = ptr_to_compat(from->si_addr); > @@ -3305,6 +3314,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > #endif > to->si_lower = ptr_to_compat(from->si_lower); > to->si_upper = ptr_to_compat(from->si_upper); > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_PKUERR: > to->si_addr = ptr_to_compat(from->si_addr); > @@ -3312,6 +3322,7 @@ void copy_siginfo_to_external32(struct compat_siginfo *to, > to->si_trapno = from->si_trapno; > #endif > to->si_pkey = from->si_pkey; > + to->si_xflags = from->si_xflags; > break; > case SIL_CHLD: > to->si_pid = from->si_pid; > @@ -3370,6 +3381,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > #ifdef __ARCH_SI_TRAPNO > to->si_trapno = from->si_trapno; > #endif > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_MCEERR: > to->si_addr = compat_ptr(from->si_addr); > @@ -3377,6 +3389,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > to->si_trapno = from->si_trapno; > #endif > to->si_addr_lsb = from->si_addr_lsb; > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_BNDERR: > to->si_addr = compat_ptr(from->si_addr); > @@ -3385,6 +3398,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > #endif > to->si_lower = compat_ptr(from->si_lower); > to->si_upper = compat_ptr(from->si_upper); > + to->si_xflags = from->si_xflags; > break; > case SIL_FAULT_PKUERR: > to->si_addr = compat_ptr(from->si_addr); > @@ -3392,6 +3406,7 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to, > to->si_trapno = from->si_trapno; > #endif > to->si_pkey = from->si_pkey; > + to->si_xflags = from->si_xflags; How did you figure out the list of places to make these changes? I'm not sure how to confirm that it's exhaustive. It's a shame if we can't simply apply the change in one place. Would the refactoring be too invasive to accomplish that? Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel