linux-kernel - Re: [PATCH RFC 1/3] seccomp: add generic code for jitted seccomp filters.

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]

Date:	Fri, 15 Mar 2013 12:22:55 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Nicolas Schichan <nschichan@...ebox.fr>
Cc:	Will Drewry <wad@...omium.org>,
	Mircea Gherzan <mgherzan@...il.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Eric Paris <eparis@...hat.com>,
	James Morris <james.l.morris@...cle.com>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC 1/3] seccomp: add generic code for jitted seccomp filters.

On Fri, Mar 15, 2013 at 12:10 PM, Nicolas Schichan <nschichan@...ebox.fr> wrote:
> On 03/15/2013 07:45 PM, Kees Cook wrote:
>>
>> On Fri, Mar 15, 2013 at 11:28 AM, Nicolas Schichan <nschichan@...ebox.fr>
>> wrote:
>>>
>>> +/**
>>> + * struct seccomp_filter - container for seccomp BPF programs
>>> + *
>>> + * @usage: reference count to manage the object lifetime.
>>> + *         get/put helpers should be used when accessing an instance
>>> + *         outside of a lifetime-guarded section.  In general, this
>>> + *         is only needed for handling filters shared across tasks.
>>> + * @prev: points to a previously installed, or inherited, filter
>>> + * @len: the number of instructions in the program
>>> + * @insns: the BPF program instructions to evaluate
>>
>>
>> This should be updated to include the new bpf_func field.
>
> Sure, I'll fix this in a re-spin of the patch serie.
>
>> Regardless, it'd be better to not expose this structure to userspace.
>
> Yes, I did not realise that this header was exported to userspace. Do you
> know any place not exported to userspace where the structure definition
> would be appropriate ?

Nothing really jumps to mind. :( We should probably do the uapi split,
so that this can stay here, but the public stuff is in the other file.
I'm not actually sure what's needed to do that split, though.

>>> @@ -213,7 +185,7 @@ static u32 seccomp_run_filters(int syscall)
>>>           * value always takes priority (ignoring the DATA).
>>>           */
>>>          for (f = current->seccomp.filter; f; f = f->prev) {
>>> -               u32 cur_ret = sk_run_filter(NULL, f->insns);
>>> +               u32 cur_ret = f->bpf_func(NULL, f->insns);
>>
>> This will make bpf_func a rather attractive target inside the kernel.
>> I wonder if there could be ways to harden it against potential attack.
>
> I'm not sure I follow, but is it because any user can install a SECCOMP
> filter which will trigger JIT code generation with potential JIT spraying
> attack payload ?

I actually mean that when an arbitrary write vulnerability is used
against the kernel, finding bpf_func may make a good target since it
hangs off the process stack. There are plenty of targets, but just
wonder if there would be some trivial way to handle this. Probably
not. :)

> In that case, the same problem exists with struct sk_filter->bpf_func, as
> SO_ATTACH_FILTER, with BPT JIT enabled, does not require any particular
> privilege AFAICS.

Yeah, these problems aren't unique to seccomp, unfortunately.

> Regarding JIT spraying, I believe ARM is actually worse than x86 in that
> regard, since the values appearing in the literal pool can be quite easily
> controlled by an attacker.

Yeah, same for x86, really. Masking these would be nice, but is
probably a separate discussion.

> Thanks for the review.

Sure thing! I think Will Drewry may have more suggestions as well.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/