On 02/19/2019 11:52 AM, Daniel Borkmann wrote:
[...]
> Looking at cg_skb_verifier_ops ... it seems there also a bug in the current
> code, namely that if we have a direct packet write, we don't make the skb
> writable; at that point skb->data is not private. The cg_skb_is_valid_access()
> allows to fetch PTR_TO_PACKET{,_END}, so we need a fix like the below for
> -bpf:
Ah, scratch that thought, I overlooked may_access_direct_pkt_data() prevents
writes for this prog type so not an issue.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f7d0004fc160..34fe6da0a236 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5796,6 +5796,12 @@ static bool sk_filter_is_valid_access(int off, int
> size,
> return bpf_skb_is_valid_access(off, size, type, prog, info);
> }
>
> +static int cg_skb_prologue(struct bpf_insn *insn_buf, bool direct_write,
> + const struct bpf_prog *prog)
> +{
> + return bpf_unclone_prologue(insn_buf, direct_write, prog, 0);
> +}
> +
> static bool cg_skb_is_valid_access(int off, int size,
> enum bpf_access_type type,
> const struct bpf_prog *prog,
> @@ -7595,6 +7601,7 @@ const struct bpf_verifier_ops cg_skb_verifier_ops = {
> .get_func_proto = cg_skb_func_proto,
> .is_valid_access = cg_skb_is_valid_access,
> .convert_ctx_access = bpf_convert_ctx_access,
> + .gen_prologue = cg_skb_prologue,
> };
>
> const struct bpf_prog_ops cg_skb_prog_ops = {
