On Fri, Mar 04, 2022 at 06:28:36PM +0100, Benjamin Tissoires wrote:
> When we process an incoming HID report, it is common to have to account
> for fields that are not aligned in the report. HID is using 2 helpers
> hid_field_extract() and implement() to pick up any data at any offset
> within the report.
>
> Export those 2 helpers in BPF programs so users can also rely on them.
> The second net worth advantage of those helpers is that now we can
> fetch data anywhere in the report without knowing at compile time the
> location of it. The boundary checks are done in hid-bpf.c, to prevent
> a memory leak.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
>
> ---
>
> changes in v2:
> - split the patch with libbpf and HID left outside.
> ---
> include/linux/bpf-hid.h | 4 +++
> include/uapi/linux/bpf.h | 32 ++++++++++++++++++++
> kernel/bpf/hid.c | 53 ++++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 32 ++++++++++++++++++++
> 4 files changed, 121 insertions(+)
>
> diff --git a/include/linux/bpf-hid.h b/include/linux/bpf-hid.h
> index 0c5000b28b20..69bb28523ceb 100644
> --- a/include/linux/bpf-hid.h
> +++ b/include/linux/bpf-hid.h
> @@ -93,6 +93,10 @@ struct bpf_hid_hooks {
> int (*link_attach)(struct hid_device *hdev, enum bpf_hid_attach_type type);
> void (*link_attached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
> void (*array_detached)(struct hid_device *hdev, enum bpf_hid_attach_type type);
> + int (*hid_get_data)(struct hid_device *hdev, u8 *buf, size_t buf_size,
> + u64 offset, u32 n, u8 *data, u64 data_size);
> + int (*hid_set_data)(struct hid_device *hdev, u8 *buf, size_t buf_size,
> + u64 offset, u32 n, u8 *data, u64 data_size);
> };
>
> #ifdef CONFIG_BPF
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a7a8d9cfcf24..4845a20e6f96 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5090,6 +5090,36 @@ union bpf_attr {
> * Return
> * 0 on success, or a negative error in case of failure. On error
> * *dst* buffer is zeroed out.
> + *
> + * int bpf_hid_get_data(void *ctx, u64 offset, u32 n, u8 *data, u64 size)
> + * Description
> + * Get the data of size n (in bits) at the given offset (bits) in the
> + * ctx->event.data field and store it into data.
> + *
> + * if n is less or equal than 32, we can address with bit precision,
> + * the value in the buffer. However, data must be a pointer to a u32
> + * and size must be 4.
> + *
> + * if n is greater than 32, offset and n must be a multiple of 8
> + * and the result is working with a memcpy internally.
> + * Return
> + * The length of data copied into data. On error, a negative value
> + * is returned.
> + *
> + * int bpf_hid_set_data(void *ctx, u64 offset, u32 n, u8 *data, u64 size)
> + * Description
> + * Set the data of size n (in bits) at the given offset (bits) in the
> + * ctx->event.data field.
> + *
> + * if n is less or equal than 32, we can address with bit precision,
> + * the value in the buffer. However, data must be a pointer to a u32
> + * and size must be 4.
> + *
> + * if n is greater than 32, offset and n must be a multiple of 8
> + * and the result is working with a memcpy internally.
> + * Return
> + * The length of data copied into ctx->event.data. On error, a negative
> + * value is returned.
Wait, nevermind my reviewed-by previously, see my comment about how this
might be split into 4:
bpf_hid_set_bytes()
bpf_hid_get_bytes()
bpf_hid_set_bits()
bpf_hid_get_bits()
Should be easier to understand and maintain over time, right?
thanks,
greg k-h
