Adds flag to exec events indicating a memfd target#161
Adds flag to exec events indicating a memfd target#161
Conversation
Also adds flags for whether the exec target is has setuid and/or setgid bits set
rhysre
left a comment
rhysre
left a comment
There was a problem hiding this comment.
Left a few comments.
Also I really wouldn't be comfortable merging this without some multi-kernel tests to ensure the suid/sgid/memfd detection work on all kernels we support. They should be pretty straightforward to write (just a test binary that does memfd_create and chmod as needed and some asserts to check data is correct).
Let me know if you have any issues with the test setup and I'd be happy to lend a hand.
| // memfd exec | ||
| char buf [7]; | ||
| bpf_probe_read_kernel_str(buf, 7, binprm->file->f_path.dentry->d_iname); | ||
| if (buf[0] == 'm' && buf[1] == 'e' && buf[2] == 'm' && buf[3] == 'f' && buf[4] == 'd' && buf[5] == ':' ) |
There was a problem hiding this comment.
For readability, I'd recommend just writing a 4 line implementation of strncmp in Helpers.h and using that here. You can put the string memfd: on the stack.
| ebpf_vl_field__set_size(&event->vl_fields, field, size); | ||
|
|
||
| // memfd exec | ||
| char buf [7]; |
There was a problem hiding this comment.
nit: needs code format, CI won't pass without it.
| out_cred_info("creds", &evt->creds); | ||
| out_comma(); | ||
|
|
||
| out_named_object_start("red_flags"); |
There was a problem hiding this comment.
IMO the nesting here is unnecessary, I say just keep them at the top level.
| out_comma(); | ||
| out_bool_flag("is_setuid", evt->is_setuid); | ||
| out_comma(); | ||
| out_bool_flag("is_setgid", evt->is_setgid); |
There was a problem hiding this comment.
IMO out_bool_flag should just be out_bool for simplicity and consistency with the other out_ functions.
|
Ping: @Tacklebox Can we address the review comments and merge this asap, Thanks! |
3 participants
Also adds flags for whether the exec target is has setuid and/or setgid bits set