Minor improvement ideas for user space argument preload

[andrewstrohman]

Preload creates sleep-able bpf programs that can handle page faults.

The preload mechanism only applies to user space hooks, because kernel memory is pinned. In contrast, user space memory may need to be paged in order to read data pointed to by pointer arguments.

Currently we only preload the string type. So, I took a look to see if any other types need to be preloaded. It looks like all the pointer types, except const_buf and char_buf are for kernel data structures, so they don't need to be cut over to preload. const_buf is only for tracepoints, and not directly configurable. char_buf could be cut over to preload. While it's true that char_buf can have an issue, this type requires a sizeArgIndex config companion to work, making the use case much less likely.

While experimenting with this, I was reminded that users won't be able to configure strings args unless their kernel has bpf_copy_from_user_str. It's not always the case that a page fault will block us from accessing strings, so it might be good to allow strings args (without bpf_copy_from_user_str support) only when the user specifies a new flag to force the policy to load. This way they can use string arguments without bpf_copy_from_user_str, by acknowledging the risk of malfunction by supplying this flag that allows the policy to load. An alternate idea is to only reject the policy when it's in enforcement mode, and just warn when the policy is just doing tracing.

When we resolve, we may traverse multiple pointers until we reach a terminal int type. So even though the int type by itself cannot have a page fault issue, pointers to reach it might. So, we might want to try to preload for the int types + resolve case as well. We do have an upcoming change that will detect that an argument could not be read due to page fault. But things can still be a little ambiguous when a NULL pointer is valid for the application. Nevertheless, detection of argument reading failure will help us understand if this problem is affecting the user.

Finally, I've realized that we can only preload a single argument per hook point. I think we should either error out when we detect more than one preload arg per hook point or support multiple preload args. The map that holds the preloaded read arg (sleepable_preload) is keyed by just the thread id: get_current_pid_tgid() , so it cannot accommodate preloading multiple args for that hook. We could change the key for the sleepable_preload map to include arg index, in addition to the thread ID in order to support multiple args per hook point.