On 1/22/25 9:04 PM, Maciej Żenczykowski wrote:
We're received reports of cBPF code failing to accept DHCP packets.
"BPF filter for DHCP not working (android14-6.1-lts + android-14.0.0_r74)"
I presume this is cBPF on AF_PACKET, right?
The relevant Android code is at:
https://cs.android.com/android/platform/superproject/main/+/main:packages/modules/NetworkStack/jni/network_stack_utils_jni.cpp;l=95;drc=9df50aef1fd163215dcba759045706253a5624f5
which uses a lot of macros from:
https://cs.android.com/android/platform/superproject/main/+/main:packages/modules/Connectivity/bpf/headers/include/bpf/BpfClassic.h;drc=c58cfb7c7da257010346bd2d6dcca1c0acdc8321
This is widely used and does work on the vast majority of drivers,
but is exposing a core kernel cBPF bug related to driver skb layout.
Root cause is iwlwifi driver, specifically on (at least):
Dell 7212: Intel Dual Band Wireless AC 8265
Dell 7220: Intel Wireless AC 9560
Dell 7230: Intel Wi-Fi 6E AX211
delivers frames where the UDP destination port is not in the skb linear
portion, while the cBPF code is using SKF_NET_OFF relative addressing.
simplified from above, effectively:
BPF_STMT(BPF_LDX | BPF_B | BPF_MSH, SKF_NET_OFF)
BPF_STMT(BPF_LD | BPF_H | BPF_IND, SKF_NET_OFF + 2)
BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, 68, 1, 0)
BPF_STMT(BPF_RET | BPF_K, 0)
BPF_STMT(BPF_RET | BPF_K, 0xFFFFFFFF)
fails to match udp dport=68 packets.
Specifically the 3rd cBPF instruction fails to match the condition:
if (ptr >= skb->head && ptr + size <= skb_tail_pointer(skb))
within bpf_internal_load_pointer_neg_helper() and thus returns NULL,
which results in reading -EFAULT.
This is because bpf_skb_load_helper_{8,16,32} don't include the
"data past headlen do skb_copy_bits()" logic from the non-negative
offset branch in the negative offset branch.
Note: I don't know sparc assembly, so this doesn't fix sparc...
ideally we should just delete bpf_internal_load_pointer_neg_helper()
This seems to have always been broken (but not pre-git era, since
obviously there was no eBPF helpers back then), but stuff older
than 5.4 is no longer LTS supported anyway, so using 5.4 as fixes tag.
Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx>
Cc: Willem de Bruijn <willemb@xxxxxxxxxx>
Reported-by: Matt Moeller <moeller.matt@xxxxxxxxx>
Closes: https://issuetracker.google.com/384636719 [Treble - GKI partner internal]
Signed-off-by: Maciej Żenczykowski <maze@xxxxxxxxxx>
Fixes: 219d54332a09 ("Linux 5.4")
Hm, so not strictly broken, just that using relative SKF_NET_OFF offset
is limited in that it requires the data to be in linear section. Some
potential workarounds that come to mind:
1) When you say vast majority of drivers, have you checked how much they
typically pull into linear section and whether it would make sense also
for iwlwifi to do the same? (It sounds like start of network header is
already in non-linear part for iwlwifi?)
2) Perhaps rework the filter to avoid relying on SKF_NET_OFF.. tradeoff
are more instructions, e.g.,
# tcpdump -dd udp dst port 68
{ 0x28, 0, 0, 0x0000000c },
{ 0x15, 0, 4, 0x000086dd },
{ 0x30, 0, 0, 0x00000014 },
{ 0x15, 0, 11, 0x00000011 },
{ 0x28, 0, 0, 0x00000038 },
{ 0x15, 8, 9, 0x00000044 },
{ 0x15, 0, 8, 0x00000800 },
{ 0x30, 0, 0, 0x00000017 },
{ 0x15, 0, 6, 0x00000011 },
{ 0x28, 0, 0, 0x00000014 },
{ 0x45, 4, 0, 0x00001fff },
{ 0xb1, 0, 0, 0x0000000e },
{ 0x48, 0, 0, 0x00000010 },
{ 0x15, 0, 1, 0x00000044 },
{ 0x6, 0, 0, 0x00040000 },
{ 0x6, 0, 0, 0x00000000 },
3) Use eBPF asm, e.g. you can pull in data into linear section via helper
bpf_skb_pull_data() if needed, or use bpf_skb_load_bytes() which works
for linear & non-linear data.
4) What about pulling in linear data in AF_PACKET code right before the
cBPF filter is run (perhaps usage of SKF_NET_OFF could be detected and
then only done if really needed by the filter)?
Thanks,
Daniel
