On Fri, Sep 4, 2015 at 2:06 PM, Tycho Andersen <tycho.ander...@canonical.com> wrote: > On Fri, Sep 04, 2015 at 01:34:12PM -0700, Kees Cook wrote: >> On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen >> <tycho.ander...@canonical.com> wrote: >> > +static const struct bpf_func_proto * >> > +seccomp_func_proto(enum bpf_func_id func_id) >> > +{ >> > + /* Right now seccomp eBPF loading doesn't support maps; seccomp >> > filters >> > + * are considered to be read-only after they're installed, so map >> > fds >> > + * probably need to be invalidated when a seccomp filter with maps >> > is >> > + * installed. >> > + * >> > + * The rest of these might be reasonable to call from seccomp, so >> > we >> > + * export them. >> > + */ >> > + switch (func_id) { >> > + case BPF_FUNC_ktime_get_ns: >> > + return &bpf_ktime_get_ns_proto; >> > + case BPF_FUNC_trace_printk: >> > + return bpf_get_trace_printk_proto(); >> > + case BPF_FUNC_get_prandom_u32: >> > + return &bpf_get_prandom_u32_proto; >> > + case BPF_FUNC_get_smp_processor_id: >> > + return &bpf_get_smp_processor_id_proto; >> > + case BPF_FUNC_tail_call: >> > + return &bpf_tail_call_proto; >> > + case BPF_FUNC_get_current_pid_tgid: >> > + return &bpf_get_current_pid_tgid_proto; >> > + case BPF_FUNC_get_current_uid_gid: >> > + return &bpf_get_current_uid_gid_proto; >> > + case BPF_FUNC_get_current_comm: >> > + return &bpf_get_current_comm_proto; >> > + default: >> > + return NULL; >> > + } >> > +} >> >> While this list is probably fine, I don't want to mix the addition of >> eBPF functions to the seccomp ABI with the CRIU changes. No function >> calls are currently possible and it should stay that way. > > Ok, I can remove them. > >> I was expecting to see a validator, similar to the existing BPF >> validator that is called when creating seccomp filters currently. Can >> we add a similar validator for new BPF_PROG_TYPE_SECCOMP? > > That's effectively what this patch does; when the eBPF is loaded via > bpf(), you tell bpf() you want a BPF_PROG_TYPE_SECCOMP, and it invokes > this validation/translation code, i.e. it uses > seccomp_is_valid_access() to check and make sure access are aligned > and inside struct seccomp_data.
What about limiting the possible instructions? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html