Skip to content

Commit 8108a77

Browse files
jrfastabdavem330
authored andcommitted
bpf: bpf_compute_data uses incorrect cb structure
SK_SKB program types use bpf_compute_data to store the end of the packet data. However, bpf_compute_data assumes the cb is stored in the qdisc layer format. But, for SK_SKB this is the wrong layer of the stack for this type. It happens to work (sort of!) because in most cases nothing happens to be overwritten today. This is very fragile and error prone. Fortunately, we have another hole in tcp_skb_cb we can use so lets put the data_end value there. Note, SK_SKB program types do not use data_meta, they are failed by sk_skb_is_valid_access(). Signed-off-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent dea6e19 commit 8108a77

3 files changed

Lines changed: 37 additions & 3 deletions

File tree

include/net/tcp.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,7 @@ struct tcp_skb_cb {
844844
__u32 key;
845845
__u32 flags;
846846
struct bpf_map *map;
847+
void *data_end;
847848
} bpf;
848849
};
849850
};

kernel/bpf/sockmap.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,14 @@ static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
9393
return rcu_dereference_sk_user_data(sk);
9494
}
9595

96+
/* compute the linear packet data range [data, data_end) for skb when
97+
* sk_skb type programs are in use.
98+
*/
99+
static inline void bpf_compute_data_end_sk_skb(struct sk_buff *skb)
100+
{
101+
TCP_SKB_CB(skb)->bpf.data_end = skb->data + skb_headlen(skb);
102+
}
103+
96104
static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
97105
{
98106
struct bpf_prog *prog = READ_ONCE(psock->bpf_verdict);
@@ -108,7 +116,7 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
108116
*/
109117
TCP_SKB_CB(skb)->bpf.map = NULL;
110118
skb->sk = psock->sock;
111-
bpf_compute_data_end(skb);
119+
bpf_compute_data_end_sk_skb(skb);
112120
preempt_disable();
113121
rc = (*prog->bpf_func)(skb, prog->insnsi);
114122
preempt_enable();
@@ -368,7 +376,7 @@ static int smap_parse_func_strparser(struct strparser *strp,
368376
* any socket yet.
369377
*/
370378
skb->sk = psock->sock;
371-
bpf_compute_data_end(skb);
379+
bpf_compute_data_end_sk_skb(skb);
372380
rc = (*prog->bpf_func)(skb, prog->insnsi);
373381
skb->sk = NULL;
374382
rcu_read_unlock();

net/core/filter.c

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4243,6 +4243,31 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
42434243
return insn - insn_buf;
42444244
}
42454245

4246+
static u32 sk_skb_convert_ctx_access(enum bpf_access_type type,
4247+
const struct bpf_insn *si,
4248+
struct bpf_insn *insn_buf,
4249+
struct bpf_prog *prog, u32 *target_size)
4250+
{
4251+
struct bpf_insn *insn = insn_buf;
4252+
int off;
4253+
4254+
switch (si->off) {
4255+
case offsetof(struct __sk_buff, data_end):
4256+
off = si->off;
4257+
off -= offsetof(struct __sk_buff, data_end);
4258+
off += offsetof(struct sk_buff, cb);
4259+
off += offsetof(struct tcp_skb_cb, bpf.data_end);
4260+
*insn++ = BPF_LDX_MEM(BPF_SIZEOF(void *), si->dst_reg,
4261+
si->src_reg, off);
4262+
break;
4263+
default:
4264+
return bpf_convert_ctx_access(type, si, insn_buf, prog,
4265+
target_size);
4266+
}
4267+
4268+
return insn - insn_buf;
4269+
}
4270+
42464271
const struct bpf_verifier_ops sk_filter_prog_ops = {
42474272
.get_func_proto = sk_filter_func_proto,
42484273
.is_valid_access = sk_filter_is_valid_access,
@@ -4301,7 +4326,7 @@ const struct bpf_verifier_ops sock_ops_prog_ops = {
43014326
const struct bpf_verifier_ops sk_skb_prog_ops = {
43024327
.get_func_proto = sk_skb_func_proto,
43034328
.is_valid_access = sk_skb_is_valid_access,
4304-
.convert_ctx_access = bpf_convert_ctx_access,
4329+
.convert_ctx_access = sk_skb_convert_ctx_access,
43054330
.gen_prologue = sk_skb_prologue,
43064331
};
43074332

0 commit comments

Comments
 (0)