-
Notifications
You must be signed in to change notification settings - Fork 78
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
Fix null termination bug #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
bpf/go_str.h
Outdated
// put in a null terminator if we are not at max_size | ||
if (size < max_size) { | ||
char null = 0; | ||
bpf_probe_read((char *)field + size, 1, &null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... Is eBPF complaining if you just try field[size+1] = 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't t try it, but I can before I merge this. I thought with reserve/commit we had to use this approach since they can't prove we are allowed to access the memory. I'll try...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that worked, I should've tried that first :).
Thanks Mario! |
Since we use the ring buffer reserve and commit strategy, we overwrite the memory directly to the ring buffer, avoiding two unnecessary operations, zero init on the stack for the structure and memcopy from the structure to the ring buffer. However, if the ring buffer gives us dirty pages of memory, we need to put a null terminator on the strings. Unlike using bpf_probe_read on a C99 initialized struct, we can't be sure we get zero initialized memory.
We only add the null terminator if we haven't written to the max size of the buffer. The go code that reads the C string looks for the null character and if it doesn't find it, it used the full array length as the size.