linux-kernel - Re: [RFC PATCH 0/3] Dealing with the aliases of SI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180419092840.GL16308@e103592.cambridge.arm.com>
Date:   Thu, 19 Apr 2018 10:28:40 +0100
From:   Dave Martin <Dave.Martin@....com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Dmitry V. Levin" <ldv@...linux.org>,
        sparclinux <sparclinux@...r.kernel.org>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        ppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 0/3] Dealing with the aliases of SI_USER

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