On Sun, Apr 15, 2018 at 11:16:04AM -0700, Linus Torvalds wrote:
[...]
> The other thing we should do is to get rid of the stupid padding.
> Right now "struct siginfo" is pointlessly padded to 128 bytes. That is
> completely insane, when it's always just zero in the kernel.
Agreed, inside the kernel the padding achieves nothing.
> So put that _pad[] thing inside #ifndef __KERNEL__, and make
> copy_siginfo_to_user() write the padding zeroes when copying to user
> space. The reason for the padding is "future expansion", so we do want
> to tell the user space that it's maybe up to 128 bytes in size, but if
> we don't fill it all, we shouldn't waste time and memory on clearing
> the padding internally.
>
> I'm certainly *hoping* nobody depends on the whole 128 bytes in
> rt_sigqueueinfo(). In theory you can fill it all (as long as si_code
> is negative), but the man-page only says "si_value", and the compat
> function doesn't copy any more than that either, so any user that
> tries to fill in more than si_value is already broken. In fact, it
> might even be worth enforcing that in rt_sigqueueinfo(), just to see
> if anybody plays any games..
[...]
Digression:
Since we don't traditionally zero the tail-padding in the user sigframe,
is there a reliable way for userspace to detect newly-added fields in
siginfo other than by having an explicit sigaction sa_flags flag to
request them? I imagine something like [1] below from the userspace
perspective.
On a separate thread, the issue of how to report syndrome information
for SIGSEGV came up [2] (such as whether the faulting instruction was a
read or a write). This information is useful (and used) by things like
userspace sanitisers and qemu. Currently, reporting this to userspace
relies on arch-specific cruft in the sigframe.
We're committed to maintaining what's already in each arch sigframe,
but it would be preferable to have a portable way of adding information
to siginfo in a generic way. si_code doesn't really work for that,
since si_codes are mutually exclusive: I can't see a way of adding
supplementary information using si_code.
Anyway, that would be a separate RFC in the future (if ever).
Cheers
---Dave
[1]
static volatile int have_extflags = 0;
static void handler(int n, siginfo_t *si, void *uc)
{
/* ... */
if (have_extflags) {
/* Check si->si_extflags */
} else {
/* fallback */
}
/* ... */
}
int main(void)
{
/* ... */
struct sigaction sa;
/* ... */
sa.sa_flags = SA_SIGINFO | SA_SIGINFO_EXTFLAGS;
sa.sa_sigaction = handler;
if (!sigaction(SIGSEGV, &sa, NULL)) {
have_extflags = 1;
} else {
sa.sa_flags &= ~SA_SIGINFO_EXTFLAGS;
if (sigaction(SIGSEGV, &sa, NULL))
goto error;
}
/* ... */
}
[2] [RFC PATCH] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/571428.html
