From: Andy Lutomirski <luto@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Sean Christopherson <sean.j.christopherson@intel.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [5.2 REGRESSION] Generic vDSO breaks seccomp-enabled userspace on i386
Date: Mon, 22 Jul 2019 16:47:36 -0700 [thread overview]
Message-ID: <CALCETrWqu-S3rrg8kf6aqqkXg9Z+TFQHbUgpZEiUU+m8KRARqg@mail.gmail.com> (raw)
In-Reply-To: <201907221620.F31B9A082@keescook>
On Mon, Jul 22, 2019 at 4:28 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jul 22, 2019 at 12:17:16PM -0700, Andy Lutomirski wrote:
> > On Mon, Jul 22, 2019 at 11:39 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Mon, Jul 22, 2019 at 08:31:32PM +0200, Thomas Gleixner wrote:
> > > > On Mon, 22 Jul 2019, Kees Cook wrote:
> > > > > Just so I'm understanding: the vDSO change introduced code to make an
> > > > > actual syscall on i386, which for most seccomp filters would be rejected?
> > > >
> > > > No. The old x86 specific VDSO implementation had a fallback syscall as
> > > > well, i.e. clock_gettime(). On 32bit clock_gettime() uses the y2038
> > > > endangered timespec.
> > > >
> > > > So when the VDSO was made generic we changed the internal data structures
> > > > to be 2038 safe right away. As a consequence the fallback syscall is not
> > > > clock_gettime(), it's clock_gettime64(). which seems to surprise seccomp.
> > >
> > > Okay, it's didn't add a syscall, it just changed it. Results are the
> > > same: conservative filters suddenly start breaking due to the different
> > > call. (And now I see why Andy's alias suggestion would help...)
> > >
> > > I'm not sure which direction to do with this. It seems like an alias
> > > list is a large hammer for this case, and a "seccomp-bypass when calling
> > > from vDSO" solution seems too fragile?
> > >
> >
> > I don't like the seccomp bypass at all. If someone uses seccomp to
> > disallow all clock_gettime() variants, there shouldn't be a back door
> > to learn the time.
> >
> > Here's the restart_syscall() logic that makes me want aliases: we have
> > different syscall numbers for restart_syscall() on 32-bit and 64-bit.
> > The logic to decide which one to use is dubious at best. I'd like to
> > introduce a restart_syscall2() that is identical to restart_syscall()
> > except that it has the same number on both variants.
>
> I've built a straw-man for this idea... but I have to say I don't
> like it. This can lead to really unexpected behaviors if someone
> were to have differing filters for the two syscalls. For example,
> let's say someone was doing a paranoid audit of 2038-unsafe clock usage
> and marked clock_gettime() with RET_KILL and marked clock_gettime64()
> with RET_LOG. This aliasing would make clock_gettime64() trigger with
> RET_KILL...
This particular issue is solvable:
> + /* Handle syscall aliases when result is not SECCOMP_RET_ALLOW. */
> + if (unlikely(action != SECCOMP_RET_ALLOW)) {
> + int alias;
> +
> + alias = seccomp_syscall_alias(sd->arch, sd->nr);
> + if (unlikely(alias != -1)) {
> + /* Use sd_local for an aliased syscall. */
> + if (sd != &sd_local) {
> + sd_local = *sd;
> + sd = &sd_local;
> + }
> + sd_local.nr = alias;
> +
> + /* Run again, with the alias, accepting the results. */
> + filter_ret = seccomp_run_filters(sd, &match);
> + data = filter_ret & SECCOMP_RET_DATA;
> + action = filter_ret & SECCOMP_RET_ACTION_FULL;
How about:
new_data = ...;
new_action = ...;
if (new_action == SECCOMP_RET_ALLOW) {
data = new_data;
action = new_action;
}
It might also be nice to allow a filter to say "hey, I want to set
this result and I do *not* want compatibility aliases applied", but
I'm not quite sure how to express that.
I don't love this whole concept, but I also don't have a better idea.
--Andy
next prev parent reply other threads:[~2019-07-22 23:47 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-19 17:03 [5.2 REGRESSION] Generic vDSO breaks seccomp-enabled userspace on i386 Sean Christopherson
2019-07-19 17:40 ` Andy Lutomirski
2019-07-22 17:16 ` Kees Cook
2019-07-22 18:31 ` Thomas Gleixner
2019-07-22 18:39 ` Kees Cook
2019-07-22 19:17 ` Andy Lutomirski
2019-07-22 23:28 ` Kees Cook
2019-07-22 23:47 ` Andy Lutomirski [this message]
2019-07-23 9:18 ` Peter Zijlstra
2019-07-23 14:04 ` Andy Lutomirski
2019-07-23 15:14 ` Peter Zijlstra
2019-07-23 21:55 ` Kees Cook
2019-07-23 22:55 ` Andy Lutomirski
2019-07-23 22:59 ` Thomas Gleixner
2019-07-23 23:43 ` Kees Cook
2019-07-23 23:56 ` Thomas Gleixner
2019-07-26 18:01 ` Sean Christopherson
2019-07-27 17:49 ` Andy Lutomirski
2019-07-27 18:43 ` Thomas Gleixner
2019-07-27 21:52 ` Thomas Gleixner
2019-07-28 0:33 ` Andy Lutomirski
2019-07-28 9:57 ` Arnd Bergmann
2019-07-28 10:30 ` Thomas Gleixner
2019-07-28 15:27 ` Arnd Bergmann
2019-07-28 18:14 ` Thomas Gleixner
2019-07-28 18:16 ` Thomas Gleixner
2019-07-28 11:13 ` Thomas Gleixner
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=CALCETrWqu-S3rrg8kf6aqqkXg9Z+TFQHbUgpZEiUU+m8KRARqg@mail.gmail.com \
--to=luto@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sean.j.christopherson@intel.com \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=x86@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).