Skip to content

Commit d4bac02

Browse files
wdebruijAlexei Starovoitov
authored andcommitted
bpf: support SKF_NET_OFF and SKF_LL_OFF on skb frags
Classic BPF socket filters with SKB_NET_OFF and SKB_LL_OFF fail to read when these offsets extend into frags. This has been observed with iwlwifi and reproduced with tun with IFF_NAPI_FRAGS. The below straightforward socket filter on UDP port, applied to a RAW socket, will silently miss matching packets. const int offset_proto = offsetof(struct ip6_hdr, ip6_nxt); const int offset_dport = sizeof(struct ip6_hdr) + offsetof(struct udphdr, dest); struct sock_filter filter_code[] = { BPF_STMT(BPF_LD + BPF_B + BPF_ABS, SKF_AD_OFF + SKF_AD_PKTTYPE), BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, PACKET_HOST, 0, 4), BPF_STMT(BPF_LD + BPF_B + BPF_ABS, SKF_NET_OFF + offset_proto), BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, IPPROTO_UDP, 0, 2), BPF_STMT(BPF_LD + BPF_H + BPF_ABS, SKF_NET_OFF + offset_dport), This is unexpected behavior. Socket filter programs should be consistent regardless of environment. Silent misses are particularly concerning as hard to detect. Use skb_copy_bits for offsets outside linear, same as done for non-SKF_(LL|NET) offsets. Offset is always positive after subtracting the reference threshold SKB_(LL|NET)_OFF, so is always >= skb_(mac|network)_offset. The sum of the two is an offset against skb->data, and may be negative, but it cannot point before skb->head, as skb_(mac|network)_offset would too. This appears to go back to when frag support was introduced to sk_run_filter in linux-2.4.4, before the introduction of git. The amount of code change and 8/16/32 bit duplication are unfortunate. But any attempt I made to be smarter saved very few LoC while complicating the code. Fixes: 1da177e ("Linux-2.6.12-rc2") Link: https://lore.kernel.org/netdev/[email protected]/ Link: https://elixir.bootlin.com/linux/2.4.4/source/net/core/filter.c#L244 Reported-by: Matt Moeller <[email protected]> Co-developed-by: Maciej Żenczykowski <[email protected]> Signed-off-by: Maciej Żenczykowski <[email protected]> Signed-off-by: Willem de Bruijn <[email protected]> Acked-by: Stanislav Fomichev <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 9bae8f4 commit d4bac02

File tree

1 file changed

+44
-36
lines changed

1 file changed

+44
-36
lines changed

net/core/filter.c

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -218,24 +218,36 @@ BPF_CALL_3(bpf_skb_get_nlattr_nest, struct sk_buff *, skb, u32, a, u32, x)
218218
return 0;
219219
}
220220

221+
static int bpf_skb_load_helper_convert_offset(const struct sk_buff *skb, int offset)
222+
{
223+
if (likely(offset >= 0))
224+
return offset;
225+
226+
if (offset >= SKF_NET_OFF)
227+
return offset - SKF_NET_OFF + skb_network_offset(skb);
228+
229+
if (offset >= SKF_LL_OFF && skb_mac_header_was_set(skb))
230+
return offset - SKF_LL_OFF + skb_mac_offset(skb);
231+
232+
return INT_MIN;
233+
}
234+
221235
BPF_CALL_4(bpf_skb_load_helper_8, const struct sk_buff *, skb, const void *,
222236
data, int, headlen, int, offset)
223237
{
224-
u8 tmp, *ptr;
238+
u8 tmp;
225239
const int len = sizeof(tmp);
226240

227-
if (offset >= 0) {
228-
if (headlen - offset >= len)
229-
return *(u8 *)(data + offset);
230-
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
231-
return tmp;
232-
} else {
233-
ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
234-
if (likely(ptr))
235-
return *(u8 *)ptr;
236-
}
241+
offset = bpf_skb_load_helper_convert_offset(skb, offset);
242+
if (offset == INT_MIN)
243+
return -EFAULT;
237244

238-
return -EFAULT;
245+
if (headlen - offset >= len)
246+
return *(u8 *)(data + offset);
247+
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
248+
return tmp;
249+
else
250+
return -EFAULT;
239251
}
240252

241253
BPF_CALL_2(bpf_skb_load_helper_8_no_cache, const struct sk_buff *, skb,
@@ -248,21 +260,19 @@ BPF_CALL_2(bpf_skb_load_helper_8_no_cache, const struct sk_buff *, skb,
248260
BPF_CALL_4(bpf_skb_load_helper_16, const struct sk_buff *, skb, const void *,
249261
data, int, headlen, int, offset)
250262
{
251-
__be16 tmp, *ptr;
263+
__be16 tmp;
252264
const int len = sizeof(tmp);
253265

254-
if (offset >= 0) {
255-
if (headlen - offset >= len)
256-
return get_unaligned_be16(data + offset);
257-
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
258-
return be16_to_cpu(tmp);
259-
} else {
260-
ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
261-
if (likely(ptr))
262-
return get_unaligned_be16(ptr);
263-
}
266+
offset = bpf_skb_load_helper_convert_offset(skb, offset);
267+
if (offset == INT_MIN)
268+
return -EFAULT;
264269

265-
return -EFAULT;
270+
if (headlen - offset >= len)
271+
return get_unaligned_be16(data + offset);
272+
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
273+
return be16_to_cpu(tmp);
274+
else
275+
return -EFAULT;
266276
}
267277

268278
BPF_CALL_2(bpf_skb_load_helper_16_no_cache, const struct sk_buff *, skb,
@@ -275,21 +285,19 @@ BPF_CALL_2(bpf_skb_load_helper_16_no_cache, const struct sk_buff *, skb,
275285
BPF_CALL_4(bpf_skb_load_helper_32, const struct sk_buff *, skb, const void *,
276286
data, int, headlen, int, offset)
277287
{
278-
__be32 tmp, *ptr;
288+
__be32 tmp;
279289
const int len = sizeof(tmp);
280290

281-
if (likely(offset >= 0)) {
282-
if (headlen - offset >= len)
283-
return get_unaligned_be32(data + offset);
284-
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
285-
return be32_to_cpu(tmp);
286-
} else {
287-
ptr = bpf_internal_load_pointer_neg_helper(skb, offset, len);
288-
if (likely(ptr))
289-
return get_unaligned_be32(ptr);
290-
}
291+
offset = bpf_skb_load_helper_convert_offset(skb, offset);
292+
if (offset == INT_MIN)
293+
return -EFAULT;
291294

292-
return -EFAULT;
295+
if (headlen - offset >= len)
296+
return get_unaligned_be32(data + offset);
297+
if (!skb_copy_bits(skb, offset, &tmp, sizeof(tmp)))
298+
return be32_to_cpu(tmp);
299+
else
300+
return -EFAULT;
293301
}
294302

295303
BPF_CALL_2(bpf_skb_load_helper_32_no_cache, const struct sk_buff *, skb,

0 commit comments

Comments
 (0)