netdev - Re: [PATCH net-next v6 01/11] bpf: Add eBPF program subtype and is_valid

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]

Message-ID: <CAGXu5j+vRGFvJZmjtAcT8Hi8B+Wz0e1b6VKYZHfQP_=DXzC4CQ@mail.gmail.com>
Date:   Tue, 18 Apr 2017 14:48:00 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Mickaël Salaün <mic@...ikod.net>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Andy Lutomirski <luto@...capital.net>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Drysdale <drysdale@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        James Morris <james.l.morris@...cle.com>,
        Jann Horn <jann@...jh.net>, Jonathan Corbet <corbet@....net>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Paul Moore <paul@...l-moore.com>,
        Sargun Dhillon <sargun@...gun.me>,
        "Serge E . Hallyn" <serge@...lyn.com>,
        Shuah Khan <shuah@...nel.org>, Tejun Heo <tj@...nel.org>,
        Thomas Graf <tgraf@...g.ch>, Will Drewry <wad@...omium.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        Linux API <linux-api@...r.kernel.org>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v6 01/11] bpf: Add eBPF program subtype and
 is_valid_subtype() verifier

On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic@...ikod.net> wrote:
> The goal of the program subtype is to be able to have different static
> fine-grained verifications for a unique program type.
>
> The struct bpf_verifier_ops gets a new optional function:
> is_valid_subtype(). This new verifier is called at the beginning of the
> eBPF program verification to check if the (optional) program subtype is
> valid.
>
> For now, only Landlock eBPF programs are using a program subtype (see
> next commit) but this could be used by other program types in the future.
>
> Changes since v5:
> * use a prog_subtype pointer and make it future-proof
> * add subtype test
> * constify bpf_load_program()'s subtype argument
> * cleanup subtype initialization
> * rebase
>
> Changes since v4:
> * replace the "status" field with "version" (more generic)
> * replace the "access" field with "ability" (less confusing)
>
> Changes since v3:
> * remove the "origin" field
> * add an "option" field
> * cleanup comments
>
> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David S. Miller <davem@...emloft.net>
> Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
> ---
> [...]
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c35ebfe6d84d..3d07b10ade5e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -843,6 +879,26 @@ static int bpf_prog_load(union bpf_attr *attr)
>         if (err < 0)
>                 goto free_prog;
>
> +       /* copy eBPF program subtype from user space */
> +       if (attr->prog_subtype) {
> +               __u32 size;
> +
> +               size = check_user_buf((void __user *)attr->prog_subtype,
> +                                     attr->prog_subtype_size,
> +                                     sizeof(prog->subtype));
> +               if (size < 0) {
> +                       err = size;
> +                       goto free_prog;
> +               }
> +               /* prog->subtype is __GFP_ZERO */
> +               if (copy_from_user(&prog->subtype,
> +                                  u64_to_user_ptr(attr->prog_subtype), size)
> +                                  != 0)

It might be worth adding a comment here about how the ToCToU of the
check-then-copy doesn't matter in this case, since it's just a
future-proofing of bits, etc.

-Kees

-- 
Kees Cook
Pixel Security