From: Daniel Borkmann <daniel@iogearbox•net>
To: David Ahern <dsa@cumulusnetworks•com>, netdev@vger•kernel.org
Cc: daniel@zonque•org, ast@fb•com
Subject: Re: [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications
Date: Wed, 26 Oct 2016 01:28:24 +0200 [thread overview]
Message-ID: <580FEA98.1090809@iogearbox.net> (raw)
In-Reply-To: <1477434613-3169-3-git-send-email-dsa@cumulusnetworks.com>
On 10/26/2016 12:30 AM, David Ahern wrote:
> Add new cgroup based program type, BPF_PROG_TYPE_CGROUP_SOCK. Similar to
> BPF_PROG_TYPE_CGROUP_SKB programs can be attached to a cgroup and run
> any time a process in the cgroup opens an AF_INET or AF_INET6 socket.
> Currently only sk_bound_dev_if is exported to userspace for modification
> by a bpf program.
>
> This allows a cgroup to be configured such that AF_INET{6} sockets opened
> by processes are automatically bound to a specific device. In turn, this
> enables the running of programs that do not support SO_BINDTODEVICE in a
> specific VRF context / L3 domain.
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks•com>
[...]
> @@ -524,6 +535,10 @@ struct bpf_tunnel_key {
> __u32 tunnel_label;
> };
>
> +struct bpf_sock {
> + __u32 bound_dev_if;
> +};
> +
> /* User return codes for XDP prog type.
> * A valid XDP program must return one of these defined values. All other
> * return codes are reserved for future use. Unknown return codes will result
[...]
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4552b8c93b99..775802881b01 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2482,6 +2482,27 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
> .arg5_type = ARG_CONST_STACK_SIZE,
> };
>
> +BPF_CALL_3(bpf_sock_store_u32, struct sock *, sk, u32, offset, u32, val)
> +{
> + u8 *ptr = (u8 *)sk;
> +
> + if (unlikely(offset > sizeof(*sk)))
> + return -EFAULT;
> +
> + *((u32 *)ptr) = val;
> +
> + return 0;
> +}
Seems strange to me. So, this helper allows to overwrite arbitrary memory
of a struct sock instance. Potentially we could crash the kernel.
And in your sock_filter_convert_ctx_access(), you already implement inline
read/write for the context ...
Your demo code does in pseudocode:
r1 = sk
r2 = offsetof(struct bpf_sock, bound_dev_if)
r3 = idx
r1->sk_bound_dev_if = idx
sock_store_u32(r1, r2, r3) // updates sk_bound_dev_if again to idx
return 1
Dropping that helper from the patch, the only thing a program can do here
is to read/write the sk_bound_dev_if helper per cgroup. Hmm ... dunno. So
this really has to be for cgroups v2, right?
next prev parent reply other threads:[~2016-10-25 23:28 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-25 22:30 [PATCH net-next 0/3] Add bpf support to set sk_bound_dev_if David Ahern
2016-10-25 22:30 ` [PATCH net-next 1/3] bpf: Refactor cgroups code in prep for new type David Ahern
2016-10-25 23:01 ` Daniel Borkmann
2016-10-25 23:04 ` David Ahern
2016-10-25 22:30 ` [PATCH net-next 2/3] bpf: Add new cgroups prog type to enable sock modifications David Ahern
2016-10-25 23:28 ` Daniel Borkmann [this message]
2016-10-26 1:55 ` Alexei Starovoitov
2016-10-26 2:38 ` David Ahern
2016-10-26 2:05 ` David Ahern
2016-10-26 8:33 ` Daniel Borkmann
2016-10-26 15:44 ` David Ahern
[not found] ` <CAF2d9jhE0OHgWrDfHwYzRk2tDbnmK_=ZdgFd2-ccpbTjdQzqmQ@mail.gmail.com>
2016-10-26 20:42 ` David Ahern
2016-10-25 23:39 ` Eric Dumazet
2016-10-26 2:21 ` David Ahern
2016-10-26 2:48 ` Eric Dumazet
2016-10-26 3:09 ` David Ahern
2016-10-26 8:41 ` Thomas Graf
2016-10-26 16:08 ` David Ahern
2016-10-26 18:57 ` Thomas Graf
2016-10-25 22:30 ` [PATCH net-next 3/3] samples: bpf: add userspace example for modifying sk_bound_dev_if David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=580FEA98.1090809@iogearbox.net \
--to=daniel@iogearbox$(echo .)net \
--cc=ast@fb$(echo .)com \
--cc=daniel@zonque$(echo .)org \
--cc=dsa@cumulusnetworks$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox