ebpf-verifier icon indicating copy to clipboard operation
ebpf-verifier copied to clipboard

Add support for static callback functions (used by bpf_for_each_map_elem, bpf_user_ringbuf_drain, bpf_timer_set_callback)

Open Alan-Jowett opened this issue 1 year ago • 8 comments

Linux supports passing BPF functions to helpers so that the runtime can then later invoke the static function.

How would we support this in the verifier.

Alan-Jowett avatar Nov 18 '24 17:11 Alan-Jowett

I think we need to

  1. Verify the callback function assuming its declared signature (found in the BTF information?). I don't see it as too different from the current verifiction, but maybe I'm wrong.
  2. Track the type of the function, to make sure the actual argument is of the right type. It doesn't work with how the current types work, but I have plans to change that.

What do you think?

elazarg avatar Nov 18 '24 19:11 elazarg

It's a little bit trickier as the callback_fn is defined as: long (*callback_fn)(struct bpf_map *map, const void *key, void *value, void *ctx);

So, we would need to validate helper function based on it's call site (i.e. we don't know the expected map key and value size until we know its caller).

https://github.com/isovalent/ebpf-docs/blob/master/docs/linux/helper-function/bpf_timer_set_callback.md

Alan-Jowett avatar Nov 18 '24 23:11 Alan-Jowett

Thinking about this more, one approach would be to verify this as if it where just a local call to the callback function. Then we wouldn't need to know the type for the helper function (anymore than we need to know the types for local calls)

Alan-Jowett avatar Nov 18 '24 23:11 Alan-Jowett

Looks like we know the map because the verifier should make sure was initialized by bpf_timer_init(). There are requirements that are not easy to find, and some information may pass from the verifier to the runtime.

elazarg avatar Nov 19 '24 00:11 elazarg

The specifications for each function must be encoded in its signature. I don't want to end up with spec scattered all over the place. This is the reason why we have signatures and assertion extraction.

elazarg avatar Nov 19 '24 00:11 elazarg

One challenge is that its signature is not well defined, but is rather defined by the call site. I.e. without knowing what map it is, we can't know the size of key and value, but we don't know the map until it's used.

Alan-Jowett avatar Nov 19 '24 00:11 Alan-Jowett

This specific challenge can be solved by analyzing the possible map size. I don't think it's difficult, but I'm not sure we want to enter the rabbit hole of inferring function signature, though when possible it's better than inlining, performance-wise.

elazarg avatar Nov 19 '24 00:11 elazarg

Maybe if we require that callbacks are strongly typed we can use the BTF data.

I.e. don't allow: void callback(void* map, void* key, void* value)

And instead require: void callback(struct some_map_type* map, key_type* key, value_type* value)

Then when the callback is used we can check that sizeof(key_type) == size of key in map etc.

This will mean that we will disallow reusing the same function with different map types (something that could technically be legal).

Alan-Jowett avatar Dec 03 '24 17:12 Alan-Jowett