netdev - Re: [PATCH bpf 7/7] bpf: make direct packet write unclone more robust

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: <CAPhsuW4V46Hi=q_fSma3mgji_JQxhjjpa1hPEtEGjE_WvEamhg@mail.gmail.com>
Date:   Wed, 24 Oct 2018 14:42:14 -0700
From:   Song Liu <liu.song.a23@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf 7/7] bpf: make direct packet write unclone more robust

On Wed, Oct 24, 2018 at 1:06 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> Given this seems to be quite fragile and can easily slip through the
> cracks, lets make direct packet write more robust by requiring that
> future program types which allow for such write must provide a prologue
> callback. In case of XDP and sk_msg it's noop, thus add a generic noop
> handler there. The latter starts out with NULL data/data_end unconditionally
> when sg pages are shared.
>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Acked-by: Alexei Starovoitov <ast@...nel.org>
> ---
>  kernel/bpf/verifier.c |  6 +++++-
>  net/core/filter.c     | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5fc9a65..171a2c8 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5709,7 +5709,11 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>         bool is_narrower_load;
>         u32 target_size;
>
> -       if (ops->gen_prologue) {
> +       if (ops->gen_prologue || env->seen_direct_write) {
> +               if (!ops->gen_prologue) {
> +                       verbose(env, "bpf verifier is misconfigured\n");
> +                       return -EINVAL;
> +               }

nit: how about this?

diff --git i/kernel/bpf/verifier.c w/kernel/bpf/verifier.c
index 6fbe7a8afed7..d35078024e35 100644
--- i/kernel/bpf/verifier.c
+++ w/kernel/bpf/verifier.c
@@ -5286,6 +5286,11 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env)
        bool is_narrower_load;
        u32 target_size;

+       if (!ops->gen_prologue && env->seen_direct_write) {
+               verbose(env, "bpf verifier is misconfigured\n");
+               return -EINVAL;
+       }
+
        if (ops->gen_prologue) {
                cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
                                        env->prog);