Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200923232923.3142503-1-keescook@chromium.org>
 <20200923232923.3142503-4-keescook@chromium.org> <CAG48ez0d80fOSTyn5QbH33WPz5UkzJJOo+V8of7YMR8pVQxumw@mail.gmail.com>
 <202009240018.A4D8274F@keescook> <CAG48ez1MWhrtkbWTNpc1v-WqWYiLM_JrCKvuE6DdH6vBY3MJzQ@mail.gmail.com>
 <ec31caaea19247f0b9bd9c73ccaa7dbd@AcuMS.aculab.com>
In-Reply-To: <ec31caaea19247f0b9bd9c73ccaa7dbd@AcuMS.aculab.com>
From:   Jann Horn <jannh@google.com>
Date:   Thu, 24 Sep 2020 14:56:19 +0200
Message-ID: <CAG48ez0n-oqivqDCFjHo1UOuNN3HnP+mi2UGB_Xqf3TwwO+ZuQ@mail.gmail.com>
Subject: Re: [PATCH 3/6] seccomp: Implement constant action bitmaps
To:     David Laight <David.Laight@aculab.com>
Cc:     Kees Cook <keescook@chromium.org>,
        YiFei Zhu <yifeifz2@illinois.edu>,
        Christian Brauner <christian.brauner@ubuntu.com>,
        Tycho Andersen <tycho@tycho.pizza>,
        Andy Lutomirski <luto@amacapital.net>,
        Will Drewry <wad@chromium.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Giuseppe Scrivano <gscrivan@redhat.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Dimitrios Skarlatos <dskarlat@cs.cmu.edu>,
        Valentin Rothberg <vrothber@redhat.com>,
        Hubertus Franke <frankeh@us.ibm.com>,
        Jack Chen <jianyan2@illinois.edu>,
        Josep Torrellas <torrella@illinois.edu>,
        Tianyin Xu <tyxu@illinois.edu>, bpf <bpf@vger.kernel.org>,
        Linux Containers <containers@lists.linux-foundation.org>,
        Linux API <linux-api@vger.kernel.org>,
        kernel list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 24, 2020 at 2:37 PM David Laight <David.Laight@aculab.com> wrote:
> From: Jann Horn
> > Sent: 24 September 2020 13:29
> ...
> > I think our goal here should be that if a syscall is always allowed,
> > seccomp should execute the smallest amount of instructions we can get
> > away with, and touch the smallest amount of memory possible (and
> > preferably that memory should be shared between threads). The bitmap
> > fastpath should probably also avoid populate_seccomp_data().
>
> If most syscalls are expected to be allowed

E.g. OpenSSH's privilege-separated network process only permits
something like 26 specific syscalls.

> then an initial:
>         if (global_mask & (1u << (syscall_number & 63))
> test can be used to skip any further lookups.

I guess that would work in principle, but I'm not convinced that it's
worth adding another layer of global caching just to avoid one load
instruction for locating the correct bitmask from the current process.
Especially when it only really provides a benefit when people use
seccomp improperly - for application sandboxing, you're supposed to
only permit a list of specific syscalls, the smaller the better.

> Although ISTR someone suggesting that the global_mask should
> be per-cpu because even shared read-only cache lines were
> expensive on some architecture.

If an architecture did make that expensive, I think we have bigger
problems to worry about than a little bitmap in seccomp. (Like the
system call table.) So I think we don't have to worry about that here.