Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map sample in ebpf-for-windows triggers "CRAB error : Integer overflow during addition" during verification #427

Closed
Alan-Jowett opened this issue Dec 1, 2022 · 5 comments · Fixed by #459

Comments

@Alan-Jowett
Copy link
Contributor

After updating to commit id 012f13f ebpf-for-windows fails during building the map BPF program sample.

bpf2c --bpf map.o --sys  >map_driver.c
Verification failed for xdp_prog with error Verification failed
Report:
exit : CRAB error : Integer overflow during addition

Failing BPF program is compiled from this source:
https://github.com/microsoft/ebpf-for-windows/blob/main/tests/sample/map.c

@Alan-Jowett
Copy link
Contributor Author

map_crab_overflow.zip

Here are two sample programs, one working the other not working.

The only difference is this line of code:

bpf_printk("bpf_map_update_elem returned %d", result);

Which expands to:

#define __bpf_printk(fmt, ...)				\
({							\
	BPF_PRINTK_FMT_MOD char ____fmt[] = fmt;	\
	bpf_trace_printk(____fmt, sizeof(____fmt),	\
			 ##__VA_ARGS__);		\
})

Which then produces something like this:

0000000000000fd8 LBB1_49:
     507:       b7 01 00 00 64 00 00 00 r1 = 100
; E:\ebpf-for-windows\tests\sample/map.c:192
     508:       6b 1a ec ff 00 00 00 00 *(u16 *)(r10 - 20) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     509:       b7 01 00 00 65 64 20 25 r1 = 622879845
; E:\ebpf-for-windows\tests\sample/map.c:192
     510:       63 1a e8 ff 00 00 00 00 *(u32 *)(r10 - 24) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     511:       18 01 00 00 59 20 72 65 00 00 00 00 74 75 72 6e r1 = 7958552634295722073 ll
; E:\ebpf-for-windows\tests\sample/map.c:192
     513:       7b 1a e0 ff 00 00 00 00 *(u64 *)(r10 - 32) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     514:       18 01 00 00 43 50 55 5f 00 00 00 00 41 52 52 41 r1 = 4706915001281368131 ll
; E:\ebpf-for-windows\tests\sample/map.c:192
     516:       7b 1a d8 ff 00 00 00 00 *(u64 *)(r10 - 40) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     517:       18 01 00 00 20 6d 61 70 00 00 00 00 20 50 45 52 r1 = 5928232584757734688 ll
; E:\ebpf-for-windows\tests\sample/map.c:192
     519:       7b 1a d0 ff 00 00 00 00 *(u64 *)(r10 - 48) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     520:       18 01 00 00 45 52 41 4c 00 00 00 00 20 66 6f 72 r1 = 8245921731643003461 ll
; E:\ebpf-for-windows\tests\sample/map.c:192
     522:       7b 1a c8 ff 00 00 00 00 *(u64 *)(r10 - 56) = r1
; E:\ebpf-for-windows\tests\sample/map.c:192
     523:       18 01 00 00 54 65 73 74 00 00 00 00 20 47 45 4e r1 = 5639992313069659476 ll
; E:\ebpf-for-windows\tests\sample/map.c:192
     525:       7b 1a c0 ff 00 00 00 00 *(u64 *)(r10 - 64) = r1

The large integers appear to cause crab to trigger an integer overflow.

@elazarg
Copy link
Collaborator

elazarg commented Dec 5, 2022

It happened to me when I first worked on the project in VMware, and I fixed it using arbitrary threshold for integer constants above which I widened to TOP. I did not keep the tests for technical reasons and apparently at some point the fix had been removed.

@Alan-Jowett
Copy link
Contributor Author

@dthaler this is the first commit it fails on:
3d2c373

This is the last commit it passed on:
67cb4d1

@Alan-Jowett
Copy link
Contributor Author

I think what saved this previously is that it always cast immediate values to int, truncating them to 32bit whereas the code now maintains the full fidelity.

@elazarg
Copy link
Collaborator

elazarg commented Dec 6, 2022

Maybe that too. I believe we are talking about very different points in time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants