From: Dave Martin <Dave.Martin@arm•com>
To: "Eric W. Biederman" <ebiederm@xmission•com>
Cc: linux-arch@vger•kernel.org,
Linus Torvalds <torvalds@linux-foundation•org>,
Linux Kernel Mailing List <linux-kernel@vger•kernel.org>,
"Dmitry V. Levin" <ldv@altlinux•org>,
sparclinux <sparclinux@vger•kernel.org>,
Russell King - ARM Linux <linux@armlinux•org.uk>,
ppc-dev <linuxppc-dev@lists•ozlabs.org>,
linux-arm-kernel <linux-arm-kernel@lists•infradead.org>
Subject: Re: [RFC PATCH 1/3] signal: Ensure every siginfo we send has all bits initialized
Date: Thu, 19 Apr 2018 09:26:57 +0100 [thread overview]
Message-ID: <20180419082656.GK16308@e103592.cambridge.arm.com> (raw)
In-Reply-To: <87h8o8y4q6.fsf@xmission.com>
On Wed, Apr 18, 2018 at 09:22:09AM -0500, Eric W. Biederman wrote:
> Dave Martin <Dave.Martin@arm•com> writes:
>
> > On Tue, Apr 17, 2018 at 02:37:38PM -0500, Eric W. Biederman wrote:
[...]
> >> My intention is to leave 0 instances of clear_siginfo in the
> >> architecture specific code. Ideally struct siginfo will be limited to
> >> kernel/signal.c but I am not certain I can quite get that far.
> >> The function do_coredump appears to have a legit need for siginfo.
> >
> > So, you mean we can't detect that the caller didn't initialise all the
> > members, or initialised the wrong union member?
>
> Correct. Even when we smuggled the the union member in the upper bits
> of si_code we got it wrong. So an interface that helps out and does
> more and is harder to misues looks desirable.
>
> > What would be the alternative? Have a separate interface for each SIL_
> > type, with only kernel/signal.c translating that into the siginfo_t that
> > userspace sees?
>
> Yes. It really isn't bad as architecture specific code only generates
> faults. In general faults only take a pointer. I have already merged
> the needed helpers into kernel/signal.c
Good point. I hadn't considered that only one class of signal comes
from the arch code, but now that you point it out, it sounds right.
> > Either way, I don't see how we force the caller to initilise the whole
> > structure.
>
> In general the plan is to convert the callers to call force_sig_fault,
> and then there is no need to have siginfo in the architecture specific
> code. I have all of the necessary helpers are already merged into
> kernel/signal.c
Makes sense.
I wonder if all the relevant siginfo data could be passed to
force_sig_fault() (or whatever) as arguments. Then the problem of
uninitialised fields goes away. Perhaps that's what you had in mind.
[...]
> >> Unless gcc has changed it's stance on type-punning through unions
> >> or it's semantics with -fno-strict_aliasing we should be good.
> >
> > In practice you're probably right.
> >
> > Today, gcc is pretty conservative in this area, and I haven't been able
> > to convince clang to optimise away memset in this way either.
> >
> > My concern is that is this assumption turns out to be wrong it may be
> > some time before anybody notices, because the leakage of kernel stack may
> > be the only symptom.
> >
> > I'll try to nail down a compiler guy to see if we can get a promise on
> > this at least with -fno-strict-aliasing.
> >
> >
> > I wonder whether it's worth protecting ourselves with something like:
> >
> >
> > static void clear_siginfo(siginfo_t *si)
> > {
> > asm ("" : "=m" (*si));
> > memset(si, 0, sizeof(*si));
> > asm ("" : "+m" (*si));
> > }
> >
> > Probably needs to be thought about more widely though. I guess it's out
> > of scope for this series.
>
> It is definitely a question worth asking.
I may follow it up later if I find myself at a loose end...
Cheers
---Dave
next prev parent reply other threads:[~2018-04-19 8:27 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-09 15:22 ppc compat v4.16 regression: sending SIGTRAP or SIGFPE via kill() returns wrong values in si_pid and si_uid Dmitry V. Levin
2018-04-12 1:34 ` sparc/ppc/arm compat siginfo ABI regressions: sending " Dmitry V. Levin
2018-04-12 1:45 ` Linus Torvalds
2018-04-12 9:58 ` Russell King - ARM Linux
2018-04-12 11:03 ` Dmitry V. Levin
2018-04-12 12:19 ` Russell King - ARM Linux
2018-04-12 12:49 ` Dmitry V. Levin
2018-04-12 13:14 ` Russell King - ARM Linux
2018-04-12 16:50 ` Linus Torvalds
2018-04-12 17:20 ` Russell King - ARM Linux
2018-04-12 17:22 ` Linus Torvalds
2018-04-13 9:42 ` Russell King - ARM Linux
2018-04-13 16:33 ` Linus Torvalds
2018-04-13 17:08 ` Dave Martin
2018-04-13 17:54 ` Russell King - ARM Linux
2018-04-13 18:23 ` Linus Torvalds
2018-04-13 18:45 ` Dave Martin
2018-04-13 19:53 ` Linus Torvalds
2018-04-15 13:12 ` Russell King - ARM Linux
2018-04-15 15:22 ` Eric W. Biederman
2018-04-15 15:56 ` [RFC PATCH 0/3] Dealing with the aliases of SI_USER Eric W. Biederman
2018-04-15 15:57 ` [RFC PATCH 1/3] signal: Ensure every siginfo we send has all bits initialized Eric W. Biederman
2018-04-17 13:23 ` Dave Martin
2018-04-17 19:37 ` Eric W. Biederman
2018-04-18 12:47 ` Dave Martin
2018-04-18 14:22 ` Eric W. Biederman
2018-04-19 8:26 ` Dave Martin [this message]
2018-04-15 15:58 ` [RFC PATCH 2/3] signal: Reduce copy_siginfo_to_user to just copy_to_user Eric W. Biederman
2018-04-15 15:59 ` [RFC PATCH 3/3] signal: Stop special casing TRAP_FIXME and FPE_FIXME in siginfo_layout Eric W. Biederman
2018-04-15 18:16 ` [RFC PATCH 0/3] Dealing with the aliases of SI_USER Linus Torvalds
2018-04-16 2:03 ` Eric W. Biederman
2018-04-18 17:58 ` Eric W. Biederman
2018-04-19 9:28 ` Dave Martin
2018-04-19 14:40 ` Eric W. Biederman
2018-04-13 18:35 ` sparc/ppc/arm compat siginfo ABI regressions: sending SIGFPE via kill() returns wrong values in si_pid and si_uid Dave Martin
2018-04-13 18:50 ` Russell King - ARM Linux
2018-04-13 18:56 ` Dave Martin
2018-04-12 17:35 ` Dmitry V. Levin
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=20180419082656.GK16308@e103592.cambridge.arm.com \
--to=dave.martin@arm$(echo .)com \
--cc=ebiederm@xmission$(echo .)com \
--cc=ldv@altlinux$(echo .)org \
--cc=linux-arch@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=sparclinux@vger$(echo .)kernel.org \
--cc=torvalds@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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