Thank you! That works when I write the opcodes by hand.
When using clang (6.0.0) to compile from C code, however, the register
allocator does not seem to want to reuse the register it used for the
comparison for the read, causing a verifier failure.
Is there a good method for getting clang to reuse the register it used
for the comparison for the read?
===== BEGIN CODE =====
#include <linux/bpf.h>
#include <linux/if_ether.h>
#include <linux/ip.h>
static int (*bpf_trace_printk)(const char* fmt, int fmt_size, ...) =
(void *)BPF_FUNC_trace_printk;
int xdp_program(struct xdp_md *context) {
void* frame_end = (void *)(long)context->data_end;
void *frame_start = (void*)(long)context->data;
if (frame_start + sizeof(struct ethhdr) + sizeof(struct iphdr) > frame_end) {
return XDP_PASS;
}
if (__builtin_bswap16(((struct ethhdr*)frame_start)->h_proto) != ETH_P_IP) {
return XDP_PASS;
}
__u16 payload_length = __builtin_bswap16(((struct iphdr*)(frame_start
+ sizeof(struct ethhdr)))->tot_len - sizeof(struct iphdr));
if (payload_length > 1500) {
return XDP_PASS;
}
if (frame_start + sizeof(struct ethhdr) + sizeof(struct iphdr) +
payload_length > frame_end) {
return XDP_PASS;
}
int last_byte_payload_offset = payload_length - 1;
__u8 byte = ((__u8*)frame_start + sizeof(struct ethhdr) +
sizeof(struct iphdr))[last_byte_payload_offset];
char fmt[] = "Packet's last byte was 0x%x\n";
bpf_trace_printk(fmt, sizeof(fmt), byte);
return XDP_TX;
}
char _license[] __attribute__((section("license"), used)) = "GPL";
===== END CODE =====
The above code was compiled with clang -O2 -Wall -Werror -target bpf -c -g.
When attempting to install the program, the verifier complained with
===== BEGIN VERIFIER OUTPUT =====
Verifier analysis:
0: (b7) r0 = 2
1: (61) r2 = *(u32 *)(r1 +4)
2: (61) r3 = *(u32 *)(r1 +0)
3: (bf) r1 = r3
4: (07) r1 += 34
5: (2d) if r1 > r2 goto pc+32
R0=inv2 R1=pkt(id=0,off=34,r=34,imm=0) R2=pkt_end(id=0,off=0,imm=0)
R3=pkt(id=0,off=0,r=34,imm=0) R10=fp0,call_-1
6: (71) r4 = *(u8 *)(r3 +12)
7: (71) r5 = *(u8 *)(r3 +13)
8: (67) r5 <<= 8
9: (4f) r5 |= r4
10: (55) if r5 != 0x8 goto pc+27
R0=inv2 R1=pkt(id=0,off=34,r=34,imm=0) R2=pkt_end(id=0,off=0,imm=0)
R3=pkt(id=0,off=0,r=34,imm=0) R4=inv(id=0,umax_value=255,var_off=(0x0;
0xff)) R5=inv8 R10=fp0,call_-1
11: (69) r3 = *(u16 *)(r3 +16)
12: (07) r3 += 65516
13: (dc) r3 = be16 r3
14: (25) if r3 > 0x5dc goto pc+23
R0=inv2 R1=pkt(id=0,off=34,r=34,imm=0) R2=pkt_end(id=0,off=0,imm=0)
R3=inv(id=0,umax_value=1500,var_off=(0x0; 0x7ff))
R4=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R5=inv8
R10=fp0,call_-1
15: (bf) r4 = r1
16: (0f) r4 += r3
17: (2d) if r4 > r2 goto pc+20
R0=inv2 R1=pkt(id=0,off=34,r=34,imm=0) R2=pkt_end(id=0,off=0,imm=0)
R3=inv(id=0,umax_value=1500,var_off=(0x0; 0x7ff))
R4=pkt(id=1,off=34,r=34,umax_value=1500,var_off=(0x0; 0x7ff)) R5=inv8
R10=fp0,call_-1
18: (0f) r1 += r3
19: (71) r3 = *(u8 *)(r1 -1)
invalid access to packet, off=33 size=1, R1(id=2,off=34,r=0)
R1 offset is outside of the packet
Error fetching program/map!
===== END VERIFIER OUTPUT =====
Thank you!
--Zvi
On Thu, Jun 7, 2018 at 6:24 AM, Edward Cree <ecree@xxxxxxxxxxxxxx> wrote:
> On 06/06/18 23:22, Zvi Effron wrote:
>> Hi XDPeople!
>>
>> I've been having great success with reading data from fixed offsets in
>> a packet/ethernet frame. But every time I try to read from a dynamic
>> offset (e.g. read the last byte in the packet by using the packet
>> length in the IP/UDP headers). It looks like once I add a register to
>> a packet pointer, the range is reset to 0, and no amount of
>> comparisons will restore a range.
>>
>> Looking through the verifier code, I believe I found where the range
>> is getting set to 0
>> (https://elixir.bootlin.com/linux/latest/source/kernel/bpf/verifier.c#L2695),
>> and it looks like the risk of overflow
>> (https://elixir.bootlin.com/linux/latest/source/kernel/bpf/verifier.c#L3267)
>> is why the range doesn't get updated by later comparisons.
>>
>> Is there a way to do a read from a packet at a dynamic offset? If not,
>> is that something that could potentially be added to the verifier? It
>> feels like `if (packet_start <= packet_start + offset && packet_end >
>> packet_start + offset)` is something that should be verifiable as
>> safe, even with potential overflow in `packet_start + offset`?
>>
>> Thank you!
>> --Zvi
> I think that to make this work you need to bound the dynamic offset
> before you add it to the packet pointer. So for instance if you read
> a packet length from the IP header, that's a u16 (so max. 0xffff), and
> it's then added to the offset of L4 (probably 14+20=34), giving a max.
> larger than MAX_PACKET_OFF.
>
> So you need to do something like
> if (offset < 0xffff && packet_start + offset < packet_end)
> because the verifier isn't smart enough to learn anything from an
> if (ptr_to_packet <= ptr_to_packet + offset)
> check. The trouble with the latter is that it's a piece of state that
> belongs both to the packet-pointer (which in the general case could be
> variably-offset already) and to the offset, so you'd need n² storage to
> record which (pointer, offset) pairs had been determined not to overflow.
>
> HTH,
> -Ed
|