I'm trying to loop over part of a TCP payload in an eBPF program, and I'm running into the invalid access to packet error.
I thought I was taking measures to prevent the error -- guarding the access with pointer + offset > data_end, while ensuring that offset is less than or equal to the max u16 value (MAX_PACKET_OFF) (as indicated in the eBPF verifier docs). But apparently I'm doing this wrong or it's not enough.
The program:
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_endian.h>
#define MAX_PACKET_OFF 0xFFFF
#define ETH_P_IP 0x0800
#define TC_ACT_OK 0
#define TC_ACT_SHOT 2
SEC("tc")
int handle_tc_ingress(struct __sk_buff *skb) {
void *data_end = (void *)(long) skb->data_end;
void *data = (void *)(long) skb->data;
struct ethhdr *eth_hdr = data;
if ((void *) (eth_hdr + 1) > data_end) {
return TC_ACT_OK;
}
if (eth_hdr->h_proto != bpf_htons(ETH_P_IP)) {
return TC_ACT_OK;
}
struct iphdr *ipv4_hdr = (struct iphdr *) (eth_hdr + 1);
if ((void *) (ipv4_hdr + 1) > data_end) {
return TC_ACT_OK;
}
struct tcphdr *tcp_hdr = (struct tcphdr *) ((__u8 *) ipv4_hdr + 4 * ipv4_hdr->ihl);
if ((void *) (tcp_hdr + 1) > data_end) {
return TC_ACT_OK;
}
__u8 *tcp_data = (__u8 *) tcp_hdr + 4 * tcp_hdr->doff;
if ((void *) tcp_data > data_end) {
return TC_ACT_OK;
}
// Ignore that this loop has too many iterations.
// Reducing `MAX_PACKET_OFF` doesn't help.
for (int i = 0; i < MAX_PACKET_OFF && tcp_data + i <= data_end; ++i) {
if (tcp_data[i] == ' ') {
bpf_printk("space");
return TC_ACT_SHOT;
}
}
return TC_ACT_OK;
}
The verifier output:
libbpf: prog 'handle_tc_ingress': -- BEGIN PROG LOAD LOG --
0: R1=ctx(off=0,imm=0) R10=fp0
; int handle_tc_ingress(struct __sk_buff *skb) {
0: (b7) r0 = 0 ; R0_w=0
; void *data_end = (void *)(long) skb->data_end;
1: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0)
; void *data = (void *)(long) skb->data;
2: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0)
; if ((void *) (eth_hdr + 1) > data_end) {
3: (bf) r3 = r1 ; R1_w=pkt(off=0,r=0,imm=0) R3_w=pkt(off=0,r=0,imm=0)
4: (07) r3 += 14 ; R3_w=pkt(off=14,r=0,imm=0)
; if ((void *) (eth_hdr + 1) > data_end) {
5: (2d) if r3 > r2 goto pc+37 ; R2_w=pkt_end(off=0,imm=0) R3_w=pkt(off=14,r=14,imm=0)
; if (eth_hdr->h_proto != bpf_htons(ETH_P_IP)) {
6: (69) r3 = *(u16 *)(r1 +12) ; R1_w=pkt(off=0,r=14,imm=0) R3_w=scalar(umax=65535,var_off=(0x0; 0xffff))
; if (eth_hdr->h_proto != bpf_htons(ETH_P_IP)) {
7: (55) if r3 != 0x8 goto pc+35 ; R3_w=8
8: (bf) r3 = r1 ; R1_w=pkt(off=0,r=14,imm=0) R3_w=pkt(off=0,r=14,imm=0)
9: (07) r3 += 34 ; R3=pkt(off=34,r=14,imm=0)
10: (2d) if r3 > r2 goto pc+32 ; R2=pkt_end(off=0,imm=0) R3=pkt(off=34,r=34,imm=0)
11: (bf) r4 = r1 ; R1=pkt(off=0,r=34,imm=0) R4_w=pkt(off=0,r=34,imm=0)
12: (07) r4 += 14 ; R4_w=pkt(off=14,r=34,imm=0)
; struct tcphdr *tcp_hdr = (struct tcphdr *) ((__u8 *) ipv4_hdr + 4 * ipv4_hdr->ihl);
13: (71) r3 = *(u8 *)(r4 +0) ; R3_w=scalar(umax=255,var_off=(0x0; 0xff)) R4_w=pkt(off=14,r=34,imm=0)
; struct tcphdr *tcp_hdr = (struct tcphdr *) ((__u8 *) ipv4_hdr + 4 * ipv4_hdr->ihl);
14: (67) r3 <<= 2 ; R3_w=scalar(umax=1020,var_off=(0x0; 0x3fc))
15: (57) r3 &= 60 ; R3_w=scalar(umax=60,var_off=(0x0; 0x3c))
; struct tcphdr *tcp_hdr = (struct tcphdr *) ((__u8 *) ipv4_hdr + 4 * ipv4_hdr->ihl);
16: (0f) r4 += r3 ; R3_w=Pscalar(umax=60,var_off=(0x0; 0x3c)) R4_w=pkt(id=1,off=14,r=0,umax=60,var_off=(0x0; 0x3c))
; if ((void *) (tcp_hdr + 1) > data_end) {
17: (bf) r5 = r4 ; R4_w=pkt(id=1,off=14,r=0,umax=60,var_off=(0x0; 0x3c)) R5_w=pkt(id=1,off=14,r=0,umax=60,var_off=(0x0; 0x3c))
18: (07) r5 += 20 ; R5_w=pkt(id=1,off=34,r=0,umax=60,var_off=(0x0; 0x3c))
; if ((void *) (tcp_hdr + 1) > data_end) {
19: (2d) if r5 > r2 goto pc+23 ; R2=pkt_end(off=0,imm=0) R5_w=pkt(id=1,off=34,r=34,umax=60,var_off=(0x0; 0x3c))
; __u8 *tcp_data = (__u8 *) tcp_hdr + 4 * tcp_hdr->doff;
20: (69) r5 = *(u16 *)(r4 +12) ; R4_w=pkt(id=1,off=14,r=34,umax=60,var_off=(0x0; 0x3c)) R5_w=scalar(umax=65535,var_off=(0x0; 0xffff))
; __u8 *tcp_data = (__u8 *) tcp_hdr + 4 * tcp_hdr->doff;
21: (77) r5 >>= 2 ; R5_w=scalar(umax=16383,var_off=(0x0; 0x3fff))
22: (57) r5 &= 60 ; R5_w=scalar(umax=60,var_off=(0x0; 0x3c))
; __u8 *tcp_data = (__u8 *) tcp_hdr + 4 * tcp_hdr->doff;
23: (0f) r4 += r5 ; R4=pkt(id=2,off=14,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124) R5=Pscalar(umax=60,var_off=(0x0; 0x3c))
; if ((void *) tcp_data > data_end) {
24: (2d) if r4 > r2 goto pc+18 ; R2=pkt_end(off=0,imm=0) R4=pkt(id=2,off=14,r=14,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124)
; for (int i = 0; i < MAX_PACKET_OFF && tcp_data + i <= data_end; ++i) {
25: (0f) r3 += r5 ; R3_w=Pscalar(umax=120,var_off=(0x0; 0x7c)) R5=Pscalar(umax=60,var_off=(0x0; 0x3c))
26: (0f) r1 += r3 ; R1_w=pkt(id=3,off=0,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124) R3_w=Pscalar(umax=120,var_off=(0x0; 0x7c))
27: (b7) r0 = 0 ; R0_w=0
28: (07) r1 += 14 ; R1_w=pkt(id=3,off=14,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124)
29: (b7) r3 = 0 ; R3_w=0
30: (05) goto pc+2
; for (int i = 0; i < MAX_PACKET_OFF && tcp_data + i <= data_end; ++i) {
33: (bf) r4 = r1 ; R1_w=pkt(id=3,off=14,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124) R4_w=pkt(id=3,off=14,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124)
34: (0f) r4 += r3 ; R3=P0 R4=pkt(id=3,off=14,r=0,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124)
; for (int i = 0; i < MAX_PACKET_OFF && tcp_data + i <= data_end; ++i) {
35: (2d) if r4 > r2 goto pc+7 ; R2=pkt_end(off=0,imm=0) R4=pkt(id=3,off=14,r=14,umax=120,var_off=(0x0; 0x7c),s32_max=124,u32_max=124)
; if (tcp_data[i] == ' ') {
36: (71) r4 = *(u8 *)(r4 +0)
invalid access to packet, off=14 size=1, R4(id=3,off=14,r=14)
R4 offset is outside of the packet
processed 35 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 1
-- END PROG LOAD LOG --
At first I thought the problem might be the goto pc+2 on line 30, which skips the check that i < MAX_PACKET_OFF. But the verifier knows that i is zero in this case (and given the loop unrolling, probably already knows that i < MAX_PACKET_OFF in every iteration), so I would think this would be fine.
I am not 100% certain if this is due to a off by one error in the condition or due to the way clang does the guard generation. In any case, it is easier to check that we programmed the correct behavior by moving the bounds check to inside the loop body:
for (int i = 0; i < MAX_PACKET_OFF; i++) {
__u8 *loop_data = tcp_data + i;
if (loop_data + 1 > data_end)
break;
if (loop_data == ' ') {
bpf_printk("space");
return TC_ACT_SHOT;
}
}