linux-kernel - Re: [PATCH v2 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Sep 2023 16:19:21 +0300
From:   Eduard Zingerman <eddyz87@...il.com>
To:     Justin Stitt <justinstitt@...gle.com>,
        Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Shuah Khan <shuah@...nel.org>
Cc:     linux-input@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        Benjamin Tissoires <bentiss@...nel.org>
Subject: Re: [PATCH v2 1/3] selftests/hid: ensure we can compile the tests
 on kernels pre-6.3

On Fri, 2023-09-08 at 22:22 +0000, Justin Stitt wrote:
> From: Benjamin Tissoires <bentiss@...nel.org>
> 
> For the hid-bpf tests to compile, we need to have the definition of
> struct hid_bpf_ctx. This definition is an internal one from the kernel
> and it is supposed to be defined in the generated vmlinux.h.
> 
> This vmlinux.h header is generated based on the currently running kernel
> or if the kernel was already compiled in the tree. If you just compile
> the selftests without compiling the kernel beforehand and you are running
> on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
> definition.
> 
> Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
> to force the definition of that symbol in case we don't find it in the
> BTF and also add __attribute__((preserve_access_index)) to further
> support CO-RE functionality for these tests.
> 
> Signed-off-by: Justin Stitt <justinstitt@...gle.com>
> Signed-off-by: Benjamin Tissoires <bentiss@...nel.org>
> ---
>  tools/testing/selftests/hid/progs/hid.c            |  3 --
>  .../testing/selftests/hid/progs/hid_bpf_helpers.h  | 49 ++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
> index 88c593f753b5..1e558826b809 100644
> --- a/tools/testing/selftests/hid/progs/hid.c
> +++ b/tools/testing/selftests/hid/progs/hid.c
> @@ -1,8 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright (c) 2022 Red hat */
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -#include <bpf/bpf_tracing.h>
>  #include "hid_bpf_helpers.h"
>  
>  char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> index 4fff31dbe0e7..ab3b18ba48c4 100644
> --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> @@ -5,6 +5,55 @@
>  #ifndef __HID_BPF_HELPERS_H
>  #define __HID_BPF_HELPERS_H
>  
> +/* "undefine" structs in vmlinux.h, because we "override" them below */

Hi Justin,

What you have here should work, however I still think that the trick
with "___local" suffix I refer to in [1] is a bit less hacky, e.g.:

    enum hid_report_type___local { ... };
    struct hid_bpf_ctx___local {
       __u32 index;
       const struct hid_device *hid; // this one should be in vmlinux.h with any config
       __u32 allocated_size;
       enum hid_report_type___local report_type;
       union {
           __s32 retval;
           __s32 size;
       };
    } __attribute__((preserve_access_index));
    
    enum hid_class_request___local { ... };
    enum hid_bpf_attach_flags___local { ... };
    ...
    extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
                                  unsigned int offset,


(sorry for being a bore, won't bring this up anymore).

Thanks,
Eduard

[1] https://lore.kernel.org/bpf/e99b4226bd450fedfebd4eb5c37054f032432b4f.camel@gmail.com/

> +#define hid_bpf_ctx hid_bpf_ctx___not_used
> +#define hid_report_type hid_report_type___not_used
> +#define hid_class_request hid_class_request___not_used
> +#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used
> +#include "vmlinux.h"
> +#undef hid_bpf_ctx
> +#undef hid_report_type
> +#undef hid_class_request
> +#undef hid_bpf_attach_flags
> +
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <linux/const.h>
> +
> +enum hid_report_type {
> +	HID_INPUT_REPORT		= 0,
> +	HID_OUTPUT_REPORT		= 1,
> +	HID_FEATURE_REPORT		= 2,
> +
> +	HID_REPORT_TYPES,
> +};
> +
> +struct hid_bpf_ctx {
> +	__u32 index;
> +	const struct hid_device *hid;
> +	__u32 allocated_size;
> +	enum hid_report_type report_type;
> +	union {
> +		__s32 retval;
> +		__s32 size;
> +	};
> +} __attribute__((preserve_access_index));
> +
> +enum hid_class_request {
> +	HID_REQ_GET_REPORT		= 0x01,
> +	HID_REQ_GET_IDLE		= 0x02,
> +	HID_REQ_GET_PROTOCOL		= 0x03,
> +	HID_REQ_SET_REPORT		= 0x09,
> +	HID_REQ_SET_IDLE		= 0x0A,
> +	HID_REQ_SET_PROTOCOL		= 0x0B,
> +};
> +
> +enum hid_bpf_attach_flags {
> +	HID_BPF_FLAG_NONE = 0,
> +	HID_BPF_FLAG_INSERT_HEAD = _BITUL(0),
> +	HID_BPF_FLAG_MAX,
> +};
> +
>  /* following are kfuncs exported by HID for HID-BPF */
>  extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
>  			      unsigned int offset,
>