Re: [PATCH bpf-next v2] bpf: allow BPF read access to qdisc pkt_len
On 12/03/2018 02:18 AM, Willem de Bruijn wrote: > From: Petar Penkov > > The pkt_len field in qdisc_skb_cb stores the skb length as it will > appear on the wire after segmentation. For byte accounting, this value > is more accurate than skb->len. It is computed on entry to the TC > layer, so only valid there. > > Allow read access to this field from BPF tc classifier and action > programs. The implementation is analogous to tc_classid, aside from > restricting to read access. > > To distinguish it from skb->len and self-describe export as wire_len. > > Changes v1->v2 > - Rename pkt_len to wire_len > > Signed-off-by: Petar Penkov > Signed-off-by: Vlad Dumitrescu > Signed-off-by: Willem de Bruijn Applied to bpf-next, thanks!
Re: [PATCH bpf-next v2] bpf: allow BPF read access to qdisc pkt_len
On Sun, Dec 2, 2018 at 5:18 PM Willem de Bruijn wrote: > > From: Petar Penkov > > The pkt_len field in qdisc_skb_cb stores the skb length as it will > appear on the wire after segmentation. For byte accounting, this value > is more accurate than skb->len. It is computed on entry to the TC > layer, so only valid there. > > Allow read access to this field from BPF tc classifier and action > programs. The implementation is analogous to tc_classid, aside from > restricting to read access. > > To distinguish it from skb->len and self-describe export as wire_len. > > Changes v1->v2 > - Rename pkt_len to wire_len > > Signed-off-by: Petar Penkov > Signed-off-by: Vlad Dumitrescu > Signed-off-by: Willem de Bruijn Acked-by: Song Liu > --- > include/uapi/linux/bpf.h| 1 + > net/core/filter.c | 16 +++ > tools/include/uapi/linux/bpf.h | 1 + > tools/testing/selftests/bpf/test_verifier.c | 32 + > 4 files changed, 50 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 8050caea7495..0183b8e70a9e 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2497,6 +2497,7 @@ struct __sk_buff { > __u32 data_meta; > struct bpf_flow_keys *flow_keys; > __u64 tstamp; > + __u32 wire_len; > }; > > struct bpf_tunnel_key { > diff --git a/net/core/filter.c b/net/core/filter.c > index bd0df75dc7b6..3d54af4c363d 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -5773,6 +5773,7 @@ static bool sk_filter_is_valid_access(int off, int size, > case bpf_ctx_range(struct __sk_buff, flow_keys): > case bpf_ctx_range_till(struct __sk_buff, family, local_port): > case bpf_ctx_range(struct __sk_buff, tstamp): > + case bpf_ctx_range(struct __sk_buff, wire_len): > return false; > } > > @@ -5797,6 +5798,7 @@ static bool cg_skb_is_valid_access(int off, int size, > case bpf_ctx_range(struct __sk_buff, tc_classid): > case bpf_ctx_range(struct __sk_buff, data_meta): > case bpf_ctx_range(struct __sk_buff, flow_keys): > + case bpf_ctx_range(struct __sk_buff, wire_len): > return false; > case bpf_ctx_range(struct __sk_buff, data): > case bpf_ctx_range(struct __sk_buff, data_end): > @@ -5843,6 +5845,7 @@ static bool lwt_is_valid_access(int off, int size, > case bpf_ctx_range(struct __sk_buff, data_meta): > case bpf_ctx_range(struct __sk_buff, flow_keys): > case bpf_ctx_range(struct __sk_buff, tstamp): > + case bpf_ctx_range(struct __sk_buff, wire_len): > return false; > } > > @@ -6273,6 +6276,7 @@ static bool sk_skb_is_valid_access(int off, int size, > case bpf_ctx_range(struct __sk_buff, data_meta): > case bpf_ctx_range(struct __sk_buff, flow_keys): > case bpf_ctx_range(struct __sk_buff, tstamp): > + case bpf_ctx_range(struct __sk_buff, wire_len): > return false; > } > > @@ -6360,6 +6364,7 @@ static bool flow_dissector_is_valid_access(int off, int > size, > case bpf_ctx_range(struct __sk_buff, data_meta): > case bpf_ctx_range_till(struct __sk_buff, family, local_port): > case bpf_ctx_range(struct __sk_buff, tstamp): > + case bpf_ctx_range(struct __sk_buff, wire_len): > return false; > } > > @@ -6685,6 +6690,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type > type, > bpf_target_off(struct sk_buff, > tstamp, 8, > target_size)); > + break; > + > + case offsetof(struct __sk_buff, wire_len): > + BUILD_BUG_ON(FIELD_SIZEOF(struct qdisc_skb_cb, pkt_len) != 4); > + > + off = si->off; > + off -= offsetof(struct __sk_buff, wire_len); > + off += offsetof(struct sk_buff, cb); > + off += offsetof(struct qdisc_skb_cb, pkt_len); > + *target_size = 4; > + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->src_reg, off); > } > > return insn - insn_buf; > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index 8050caea7495..0183b8e70a9e 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -2497,6 +2497,7 @@ struct __sk_buff { > __u32 data_meta; > struct bpf_flow_keys *flow_keys; > __u64 tstamp; > + __u32 wire_len; > }; > > struct bpf_tunnel_key { > diff --git a/tools/testing/selftests/bpf/test_verifier.c > b/tools/testing/selftests/bpf/test_verifier.c > index c3b038f26ece..b4b4a3f93639 100644 > --- a/tools/testing/selftests/bpf/test_verifier.c > +++ b/tools/testing/selftests/bpf/test_verifier.c > @@
[PATCH bpf-next v2] bpf: allow BPF read access to qdisc pkt_len
From: Petar Penkov The pkt_len field in qdisc_skb_cb stores the skb length as it will appear on the wire after segmentation. For byte accounting, this value is more accurate than skb->len. It is computed on entry to the TC layer, so only valid there. Allow read access to this field from BPF tc classifier and action programs. The implementation is analogous to tc_classid, aside from restricting to read access. To distinguish it from skb->len and self-describe export as wire_len. Changes v1->v2 - Rename pkt_len to wire_len Signed-off-by: Petar Penkov Signed-off-by: Vlad Dumitrescu Signed-off-by: Willem de Bruijn --- include/uapi/linux/bpf.h| 1 + net/core/filter.c | 16 +++ tools/include/uapi/linux/bpf.h | 1 + tools/testing/selftests/bpf/test_verifier.c | 32 + 4 files changed, 50 insertions(+) diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 8050caea7495..0183b8e70a9e 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2497,6 +2497,7 @@ struct __sk_buff { __u32 data_meta; struct bpf_flow_keys *flow_keys; __u64 tstamp; + __u32 wire_len; }; struct bpf_tunnel_key { diff --git a/net/core/filter.c b/net/core/filter.c index bd0df75dc7b6..3d54af4c363d 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -5773,6 +5773,7 @@ static bool sk_filter_is_valid_access(int off, int size, case bpf_ctx_range(struct __sk_buff, flow_keys): case bpf_ctx_range_till(struct __sk_buff, family, local_port): case bpf_ctx_range(struct __sk_buff, tstamp): + case bpf_ctx_range(struct __sk_buff, wire_len): return false; } @@ -5797,6 +5798,7 @@ static bool cg_skb_is_valid_access(int off, int size, case bpf_ctx_range(struct __sk_buff, tc_classid): case bpf_ctx_range(struct __sk_buff, data_meta): case bpf_ctx_range(struct __sk_buff, flow_keys): + case bpf_ctx_range(struct __sk_buff, wire_len): return false; case bpf_ctx_range(struct __sk_buff, data): case bpf_ctx_range(struct __sk_buff, data_end): @@ -5843,6 +5845,7 @@ static bool lwt_is_valid_access(int off, int size, case bpf_ctx_range(struct __sk_buff, data_meta): case bpf_ctx_range(struct __sk_buff, flow_keys): case bpf_ctx_range(struct __sk_buff, tstamp): + case bpf_ctx_range(struct __sk_buff, wire_len): return false; } @@ -6273,6 +6276,7 @@ static bool sk_skb_is_valid_access(int off, int size, case bpf_ctx_range(struct __sk_buff, data_meta): case bpf_ctx_range(struct __sk_buff, flow_keys): case bpf_ctx_range(struct __sk_buff, tstamp): + case bpf_ctx_range(struct __sk_buff, wire_len): return false; } @@ -6360,6 +6364,7 @@ static bool flow_dissector_is_valid_access(int off, int size, case bpf_ctx_range(struct __sk_buff, data_meta): case bpf_ctx_range_till(struct __sk_buff, family, local_port): case bpf_ctx_range(struct __sk_buff, tstamp): + case bpf_ctx_range(struct __sk_buff, wire_len): return false; } @@ -6685,6 +6690,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type, bpf_target_off(struct sk_buff, tstamp, 8, target_size)); + break; + + case offsetof(struct __sk_buff, wire_len): + BUILD_BUG_ON(FIELD_SIZEOF(struct qdisc_skb_cb, pkt_len) != 4); + + off = si->off; + off -= offsetof(struct __sk_buff, wire_len); + off += offsetof(struct sk_buff, cb); + off += offsetof(struct qdisc_skb_cb, pkt_len); + *target_size = 4; + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->src_reg, off); } return insn - insn_buf; diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 8050caea7495..0183b8e70a9e 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -2497,6 +2497,7 @@ struct __sk_buff { __u32 data_meta; struct bpf_flow_keys *flow_keys; __u64 tstamp; + __u32 wire_len; }; struct bpf_tunnel_key { diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index c3b038f26ece..b4b4a3f93639 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -14033,6 +14033,38 @@ static struct bpf_test tests[] = { .result_unpriv = REJECT, .result = ACCEPT, }, + { + "check wire_len is not readable by sockets", + .insns = { + BPF_LDX_MEM(BPF_W, BPF_REG_0,