invalid zero-sized read ? #1143

chenhengqi · 2023-09-30T09:36:10Z

chenhengqi
Sep 30, 2023

The following BPF program causes a verifier error if loaded by cilium/ebpf:

const volatile __u64 len = 0;

struct toa {
	__s64 opt1;
	__s64 opt2;
	__s64 opt3;
	__s64 opt4;
};

SEC("sockops")
int update_hdr(struct bpf_sock_ops *skops)
{
	struct toa toa = {};
	__u8 toa_len;

	if (len > 32)
		toa_len = 32;
	else
		toa_len = (__u8)len & 0xff;

	if (!toa_len)
		return 0;

	bpf_store_hdr_opt(skops, &toa, toa_len, 0);
	return 0;
}

like:

2023/09/30 17:23:11 ; bpf_store_hdr_opt(skops, &toa, toa_len, 0);
2023/09/30 17:23:11 14: (57) r3 &= 255                    ; R3_w=scalar(umax=255,var_off=(0x0; 0xff))
2023/09/30 17:23:11 15: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
2023/09/30 17:23:11 16: (07) r2 += -32                    ; R2_w=fp-32
2023/09/30 17:23:11 17: (b7) r4 = 0                       ; R4_w=0
2023/09/30 17:23:11 18: (85) call bpf_store_hdr_opt#143
2023/09/30 17:23:11 invalid zero-sized read
2023/09/30 17:23:11 verification time 1525 usec
2023/09/30 17:23:11 stack depth 32
2023/09/30 17:23:11 processed 18 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 1

But it's OK with bpftool/libbpf. The BPF instructions loaded is exactly the same.

The only difference I found is the BTF loaded is manipulated by cilium/ebpf (i.e. not directly from ELF object),
it might be related, not sure.

lmb · 2023-10-02T09:25:39Z

lmb
Oct 2, 2023
Maintainer

Interesting! Can you extract the full verifier log for both libbpf and c/ebpf?

5 replies

chenhengqi Oct 3, 2023
Author

cilium/ebpf:

func#0 @0
0: R1=ctx(off=0,imm=0) R10=fp0
; int update_hdr(struct bpf_sock_ops *skops)
0: (b7) r2 = 0                        ; R2_w=0
; struct toa toa = {};
1: (7b) *(u64 *)(r10 -8) = r2
last_idx 1 first_idx 0
regs=4 stack=0 before 0: (b7) r2 = 0
2: R2_w=0 R10=fp0 fp-8_w=00000000
2: (7b) *(u64 *)(r10 -16) = r2
last_idx 2 first_idx 0
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
3: R2_w=0 R10=fp0 fp-16_w=00000000
3: (7b) *(u64 *)(r10 -24) = r2
last_idx 3 first_idx 0
regs=4 stack=0 before 2: (7b) *(u64 *)(r10 -16) = r2
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
4: R2_w=0 R10=fp0 fp-24_w=00000000
4: (7b) *(u64 *)(r10 -32) = r2
last_idx 4 first_idx 0
regs=4 stack=0 before 3: (7b) *(u64 *)(r10 -24) = r2
regs=4 stack=0 before 2: (7b) *(u64 *)(r10 -16) = r2
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
5: R2_w=0 R10=fp0 fp-32_w=00000000
5: (b7) r3 = 32                       ; R3_w=32
; if (len > 32)
6: (18) r2 = 0xffff9e272a7b5510       ; R2_w=map_value(off=0,ks=4,vs=8,imm=0)
8: (79) r4 = *(u64 *)(r2 +0)          ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R4_w=scalar()
; if (len > 32)
9: (25) if r4 > 0x20 goto pc+4        ; R4_w=scalar(umax=32,var_off=(0x0; 0x3f))
; toa_len = (__u8)len & 0xff;
10: (79) r3 = *(u64 *)(r2 +0)         ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R3_w=scalar()
; toa_len = (__u8)len & 0xff;
11: (bf) r2 = r3                      ; R2_w=scalar(id=1) R3_w=scalar(id=1)
12: (57) r2 &= 255                    ; R2_w=scalar(umax=255,var_off=(0x0; 0xff))
; if (!toa_len)
13: (15) if r2 == 0x0 goto pc+5       ; R2_w=scalar(umax=255,var_off=(0x0; 0xff))
; bpf_store_hdr_opt(skops, &toa, toa_len, 0);
14: (57) r3 &= 255                    ; R3_w=scalar(umax=255,var_off=(0x0; 0xff))
15: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
16: (07) r2 += -32                    ; R2_w=fp-32
17: (b7) r4 = 0                       ; R4_w=0
18: (85) call bpf_store_hdr_opt#143
invalid zero-sized read
verification time 2020 usec
stack depth 32
processed 18 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 1

veristat/libbpf:

Processing 'sockops_bpfel.o'...
PROCESSING ./sockops_bpfel.o/update_hdr, DURATION US: 348, VERDICT: success, VERIFIER LOG:
verification time 348 usec
stack depth 32
processed 15 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 0

File             Program     Verdict  Duration (us)  Insns  States  Peak states
---------------  ----------  -------  -------------  -----  ------  -----------
sockops_bpfel.o  update_hdr  success            348     15       1            1
---------------  ----------  -------  -------------  -----  ------  -----------
Done. Processed 1 files, 0 programs. Skipped 1 files, 0 programs.

ti-mo Oct 4, 2023
Maintainer

@chenhengqi The verifier also produces a verifier log on successful loads, if requested. Not sure how to coax veristat into displaying this, you might need to play around with the arguments, or patch it if it doesn't already have the ability to dump a successful log.

Thanks!

chenhengqi Oct 5, 2023
Author

The log is attached below. I think I might find the root cause: the .rodata map should be freezed before doing a prog load.

func#0 @0
0: R1=ctx(off=0,imm=0) R10=fp0
; int update_hdr(struct bpf_sock_ops *skops)
0: (b7) r2 = 0                        ; R2_w=0
; struct toa toa = {};
1: (7b) *(u64 *)(r10 -8) = r2
last_idx 1 first_idx 0
regs=4 stack=0 before 0: (b7) r2 = 0
2: R2_w=0 R10=fp0 fp-8_w=00000000
2: (7b) *(u64 *)(r10 -16) = r2
last_idx 2 first_idx 0
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
3: R2_w=0 R10=fp0 fp-16_w=00000000
3: (7b) *(u64 *)(r10 -24) = r2
last_idx 3 first_idx 0
regs=4 stack=0 before 2: (7b) *(u64 *)(r10 -16) = r2
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
4: R2_w=0 R10=fp0 fp-24_w=00000000
4: (7b) *(u64 *)(r10 -32) = r2
last_idx 4 first_idx 0
regs=4 stack=0 before 3: (7b) *(u64 *)(r10 -24) = r2
regs=4 stack=0 before 2: (7b) *(u64 *)(r10 -16) = r2
regs=4 stack=0 before 1: (7b) *(u64 *)(r10 -8) = r2
regs=4 stack=0 before 0: (b7) r2 = 0
5: R2_w=0 R10=fp0 fp-32_w=00000000
5: (b7) r3 = 32                       ; R3_w=32
; if (len > 32)
6: (18) r2 = 0xffffa09602086000       ; R2_w=map_value(off=0,ks=4,vs=8,imm=0)
8: (79) r4 = *(u64 *)(r2 +0)          ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R4_w=0
; if (len > 32)
9: (25) if r4 > 0x20 goto pc+4
last_idx 9 first_idx 0
regs=10 stack=0 before 8: (79) r4 = *(u64 *)(r2 +0)
10: R4_w=0
; toa_len = (__u8)len & 0xff;
10: (79) r3 = *(u64 *)(r2 +0)         ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R3_w=0
; toa_len = (__u8)len & 0xff;
11: (bf) r2 = r3                      ; R2_w=0 R3_w=0
12: (57) r2 &= 255                    ; R2_w=0
; if (!toa_len)
13: (15) if r2 == 0x0 goto pc+5
last_idx 13 first_idx 0
regs=4 stack=0 before 12: (57) r2 &= 255
regs=4 stack=0 before 11: (bf) r2 = r3
regs=8 stack=0 before 10: (79) r3 = *(u64 *)(r2 +0)
19: R2=0
; }
19: (b7) r0 = 0                       ; R0_w=0
20: (95) exit
verification time 1218 usec
stack depth 32
processed 15 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 0

ti-mo Oct 5, 2023
Maintainer

How are you loading this program? Are you unsetting the MapSpec.Freeze flag? 🙂

lmb Oct 5, 2023
Maintainer

Here is the between the two logs:

--- cebpf.txt	2023-10-05 17:18:39.253271236 +0100
+++ libbpf.txt	2023-10-05 17:19:20.395849058 +0100
@@ -27,24 +27,28 @@
 5: R2_w=0 R10=fp0 fp-32_w=00000000
 5: (b7) r3 = 32                       ; R3_w=32
 ; if (len > 32)
-6: (18) r2 = 0xffff9e272a7b5510       ; R2_w=map_value(off=0,ks=4,vs=8,imm=0)
-8: (79) r4 = *(u64 *)(r2 +0)          ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R4_w=scalar()
+6: (18) r2 = 0xffffa09602086000       ; R2_w=map_value(off=0,ks=4,vs=8,imm=0)
+8: (79) r4 = *(u64 *)(r2 +0)          ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R4_w=0
 ; if (len > 32)
-9: (25) if r4 > 0x20 goto pc+4        ; R4_w=scalar(umax=32,var_off=(0x0; 0x3f))
+9: (25) if r4 > 0x20 goto pc+4
+last_idx 9 first_idx 0
+regs=10 stack=0 before 8: (79) r4 = *(u64 *)(r2 +0)
+10: R4_w=0
 ; toa_len = (__u8)len & 0xff;
-10: (79) r3 = *(u64 *)(r2 +0)         ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R3_w=scalar()
+10: (79) r3 = *(u64 *)(r2 +0)         ; R2_w=map_value(off=0,ks=4,vs=8,imm=0) R3_w=0
 ; toa_len = (__u8)len & 0xff;
-11: (bf) r2 = r3                      ; R2_w=scalar(id=1) R3_w=scalar(id=1)
-12: (57) r2 &= 255                    ; R2_w=scalar(umax=255,var_off=(0x0; 0xff))
+11: (bf) r2 = r3                      ; R2_w=0 R3_w=0
+12: (57) r2 &= 255                    ; R2_w=0
 ; if (!toa_len)
-13: (15) if r2 == 0x0 goto pc+5       ; R2_w=scalar(umax=255,var_off=(0x0; 0xff))
-; bpf_store_hdr_opt(skops, &toa, toa_len, 0);
-14: (57) r3 &= 255                    ; R3_w=scalar(umax=255,var_off=(0x0; 0xff))
-15: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
-16: (07) r2 += -32                    ; R2_w=fp-32
-17: (b7) r4 = 0                       ; R4_w=0
-18: (85) call bpf_store_hdr_opt#143
-invalid zero-sized read
-verification time 2020 usec
+13: (15) if r2 == 0x0 goto pc+5
+last_idx 13 first_idx 0
+regs=4 stack=0 before 12: (57) r2 &= 255
+regs=4 stack=0 before 11: (bf) r2 = r3
+regs=8 stack=0 before 10: (79) r3 = *(u64 *)(r2 +0)
+19: R2=0
+; }
+19: (b7) r0 = 0                       ; R0_w=0
+20: (95) exit
+verification time 1218 usec
 stack depth 32
-processed 18 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 1
+processed 15 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 0

chenhengqi · 2023-10-05T23:44:53Z

chenhengqi
Oct 5, 2023
Author

How are you loading this program? Are you unsetting the MapSpec.Freeze flag? 🙂

I didn't do anything special, just:

var obj sockopsObjects
err := loadSockopsObjects(&obj, &ebpf.CollectionOptions{
	Programs: ebpf.ProgramOptions{
		LogLevel: ebpf.LogLevelInstruction | ebpf.LogLevelBranch | ebpf.LogLevelStats,
		LogSize:  100_000_000,
	},
})

In CollectionSpec::LoadAndAssign, we do prog load and then map freeze, no ?

ebpf/collection.go

Lines 306 to 314 in 9feeb26

    
           // Load the Maps and Programs requested by the annotated struct. 
        
           if err := assignValues(to, getValue); err != nil { 
        
           	return err 
        
           } 
        
           // Populate the requested maps. Has a chance of lazy-loading other dependent maps. 
        
           if err := loader.populateMaps(); err != nil { 
        
           	return err 
        
           }

1 reply

ti-mo Oct 6, 2023
Maintainer

This ended up being a bug fixed by #1159. Thanks for reporting!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

invalid zero-sized read ? #1143

{{title}}

Replies: 2 comments 6 replies

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

invalid zero-sized read ? #1143

chenhengqi Sep 30, 2023

Replies: 2 comments · 6 replies

lmb Oct 2, 2023 Maintainer

chenhengqi Oct 3, 2023 Author

ti-mo Oct 4, 2023 Maintainer

chenhengqi Oct 5, 2023 Author

ti-mo Oct 5, 2023 Maintainer

lmb Oct 5, 2023 Maintainer

chenhengqi Oct 5, 2023 Author

ti-mo Oct 6, 2023 Maintainer

chenhengqi
Sep 30, 2023

Replies: 2 comments 6 replies

lmb
Oct 2, 2023
Maintainer

chenhengqi Oct 3, 2023
Author

ti-mo Oct 4, 2023
Maintainer

chenhengqi Oct 5, 2023
Author

ti-mo Oct 5, 2023
Maintainer

lmb Oct 5, 2023
Maintainer

chenhengqi
Oct 5, 2023
Author

ti-mo Oct 6, 2023
Maintainer