From: Dave Martin <Dave.Martin@arm.com>
To: Peter Collingbourne <pcc@google.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
linux-parisc@vger.kernel.org,
Catalin Marinas <catalin.marinas@arm.com>,
Kevin Brodsky <kevin.brodsky@arm.com>,
Oleg Nesterov <oleg@redhat.com>,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
Kostya Serebryany <kcc@google.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Andrey Konovalov <andreyknvl@google.com>,
David Spickett <david.spickett@linaro.org>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
Will Deacon <will@kernel.org>,
Evgenii Stepanov <eugenis@google.com>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH v9 5/6] signal: define the field siginfo.si_xflags
Date: Wed, 19 Aug 2020 16:40:29 +0100 [thread overview]
Message-ID: <20200819154028.GH6642@arm.com> (raw)
In-Reply-To: <e26271d2b3767cdbd265033e6f7eb28f828f3a28.1597720138.git.pcc@google.com>
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 <pcc@google.com>
> ---
> 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 <asm/sigcontext.h>
>
> 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
next prev parent reply other threads:[~2020-08-19 15:41 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-18 3:33 [PATCH v9 0/6] arm64: expose FAR_EL1 tag bits in siginfo Peter Collingbourne
2020-08-18 3:33 ` [PATCH v9 1/6] parisc: start using signal-defs.h Peter Collingbourne
2020-08-18 3:33 ` [PATCH v9 2/6] arch: move SA_* definitions to generic headers Peter Collingbourne
2020-08-19 7:13 ` Geert Uytterhoeven
2020-08-19 22:44 ` Peter Collingbourne
2020-08-19 10:30 ` Dave Martin
2020-08-19 21:35 ` Peter Collingbourne
2020-08-18 3:33 ` [PATCH v9 3/6] signal: clear non-uapi flag bits when passing/returning sa_flags Peter Collingbourne
2020-08-19 10:39 ` Dave Martin
2020-08-19 23:39 ` Peter Collingbourne
2020-08-24 13:40 ` Dave Martin
2020-08-25 0:51 ` Peter Collingbourne
2020-08-25 14:25 ` Dave Martin
2020-08-18 3:33 ` [PATCH v9 4/6] signal: define the SA_UNSUPPORTED bit in sa_flags Peter Collingbourne
2020-08-19 14:51 ` Dave Martin
2020-08-20 0:23 ` Peter Collingbourne
2020-08-24 13:41 ` Dave Martin
2020-08-18 3:33 ` [PATCH v9 5/6] signal: define the field siginfo.si_xflags Peter Collingbourne
2020-08-19 15:40 ` Dave Martin [this message]
2020-08-20 1:37 ` Peter Collingbourne
2020-08-24 14:03 ` Dave Martin
2020-08-25 1:27 ` Peter Collingbourne
2020-08-25 14:47 ` Dave Martin
2020-08-25 20:08 ` Peter Collingbourne
2020-08-26 16:15 ` Dave Martin
2020-08-18 3:33 ` [PATCH v9 6/6] arm64: expose FAR_EL1 tag bits in siginfo Peter Collingbourne
2020-08-19 15:56 ` Dave Martin
2020-08-20 1:49 ` Peter Collingbourne
2020-08-24 14:23 ` Dave Martin
2020-08-25 2:18 ` Peter Collingbourne
2020-08-25 15:02 ` Dave Martin
2020-08-25 22:06 ` Peter Collingbourne
2020-08-26 15:32 ` Dave Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200819154028.GH6642@arm.com \
--to=dave.martin@arm.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=andreyknvl@google.com \
--cc=catalin.marinas@arm.com \
--cc=david.spickett@linaro.org \
--cc=ebiederm@xmission.com \
--cc=eugenis@google.com \
--cc=kcc@google.com \
--cc=kevin.brodsky@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-parisc@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=pcc@google.com \
--cc=rth@twiddle.net \
--cc=vincenzo.frascino@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).