RE: [PATCH bpf-next v2 2/2] bpf: Replace callers of BPF_CAST_CALL with proper function typedef

From: David Laight
Date: Sat Oct 02 2021 - 12:09:25 EST


From: Kees Cook
> Sent: 29 September 2021 00:10
...
> In order to keep ahead of cases in the kernel where Control Flow
> Integrity (CFI) may trip over function call casts, enabling
> -Wcast-function-type is helpful. To that end, BPF_CAST_CALL causes
> various warnings and is one of the last places in the kernel
> triggering this warning.
...
> -static int bpf_for_each_array_elem(struct bpf_map *map, void *callback_fn,
> +static int bpf_for_each_array_elem(struct bpf_map *map, bpf_callback_t callback_fn,
> void *callback_ctx, u64 flags)
> {
> u32 i, key, num_elems = 0;
> @@ -668,9 +668,8 @@ static int bpf_for_each_array_elem(struct bpf_map *map, void *callback_fn,
> val = array->value + array->elem_size * i;
> num_elems++;
> key = i;
> - ret = BPF_CAST_CALL(callback_fn)((u64)(long)map,
> - (u64)(long)&key, (u64)(long)val,
> - (u64)(long)callback_ctx, 0);
> + ret = callback_fn((u64)(long)map, (u64)(long)&key,
> + (u64)(long)val, (u64)(long)callback_ctx, 0);
> /* return value: 0 - continue, 1 - stop and return */
> if (ret)
> break;

This is still entirely horrid and potentially error prone.
While a callback function seems a nice idea the code is
almost always better and much easier to read if some
kind of iterator function is used so that the calling
code is just a simple loop.
This is true even if you need a #define for the loop end.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)