Hi Peter,

On Mon, Nov 02, 2020 at 08:09:43PM -0800, 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_tag_bits{,_mask} fields.

A new sigcontext flag, SA_FAULTFLAGS, 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.

As per patch 5, a user is supposed to call sigaction() twice to figure
out whether _faultflags is meaningful. That's the part I'm not
particularly fond of. Are the unused parts of siginfo always zeroed when
the kernel delivers a signal? If yes, we could simply check the new
field for non-zero bits.

It is possible for an si_faultflags-unaware program to cause a signal
handler in an si_faultflags-aware program to be called with a provided
siginfo data structure by using one of the following syscalls:

- ptrace(PTRACE_SETSIGINFO)
- pidfd_send_signal
- rt_sigqueueinfo
- rt_tgsigqueueinfo

So we need to prevent the si_faultflags-unaware program from causing an
uninitialized read of si_faultflags in the si_faultflags-aware program when
it uses one of these syscalls.

The last three cases can be handled by observing that each of these
syscalls fails if si_code >= 0. We also observe that kill(2) and
tgkill(2) may be used to send a signal where si_code == 0 (SI_USER),
so we define si_faultflags to only be valid if si_code > 0.

There is no such check on si_code in ptrace(PTRACE_SETSIGINFO), so
we make ptrace(PTRACE_SETSIGINFO) clear the si_faultflags field if it
detects that the signal would use the _sigfault layout, and introduce
a new ptrace request type, PTRACE_SETSIGINFO2, that a si_faultflags-aware
program may use to opt out of this behavior.

I find this pretty fragile but maybe I have to read it a few more times
to fully understand the implications ;).

Could we instead copy all the fields, potentially uninitialised, and
instead filter them when delivering the signal based on the
SA_FAULTFLAGS? That means that the kernel only writes si_faultflags if
the user requested it.

v12:
- Change type of si_xflags to u32 to avoid increasing alignment

[...]

Sorry, I haven't checked the previous discussion on alignment here but
don't we already require 64-bit alignment because of other members in
the _sigfault union? We already have void * throughout this and with the
next patch we just have a gap (unless I miscalculated the offsets).

-- 
Catalin