linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] HID: bpf: some fixes for pre-loading HID-BPF
@ 2024-04-19 14:47 Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 1/3] HID: bpf: fix a comment in a define Benjamin Tissoires
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-19 14:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Benjamin Tissoires, stable

As I am working on the next functionalities of HID-BPF, I realized that
I had a few issues while preloading the skeleton at boot.

None of the errors are terrible as they are not inducing a kernel crash,
so it's not super urgent IMO.

Regarding the last one, I'm not sure what makes RHEL behave slightly
different than upstream. But I am not sure also that the code matches
upstream everywhere, so lazy loading it seems like a sensible idea.

Furthermore, that also means that the code will not be available until
requested by user space, which fits well in the whole idea of HID-BPF:
if the user doesn't want it, then it shouldn't be it.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Benjamin Tissoires (3):
      HID: bpf: fix a comment in a define
      HID: bpf: fix return value of entrypoints_*__attach()
      HID: bpf: lazy load the hid_tail_call entrypoint

 drivers/hid/bpf/hid_bpf_dispatch.c  |  6 ------
 drivers/hid/bpf/hid_bpf_jmp_table.c | 17 ++++++++++++-----
 2 files changed, 12 insertions(+), 11 deletions(-)
---
base-commit: b912cf042072e12e93faa874265b30cc0aa521b9
change-id: 20240419-hid_bpf_lazy_skel-ab0d674cb49b

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] HID: bpf: fix a comment in a define
  2024-04-19 14:47 [PATCH 0/3] HID: bpf: some fixes for pre-loading HID-BPF Benjamin Tissoires
@ 2024-04-19 14:47 ` Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 2/3] HID: bpf: fix return value of entrypoints_*__attach() Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint Benjamin Tissoires
  2 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-19 14:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Benjamin Tissoires, stable

It compiles fine, but my editor is troubled by this.
Makes things clearer by putting the comment above the #define, this
way we are sure it all goes well.

Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/bpf/hid_bpf_jmp_table.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index aa8e1c79cdf5..89496aabbe15 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -19,10 +19,10 @@
 #include "hid_bpf_dispatch.h"
 #include "entrypoints/entrypoints.lskel.h"
 
-#define HID_BPF_MAX_PROGS 1024 /* keep this in sync with preloaded bpf,
-				* needs to be a power of 2 as we use it as
-				* a circular buffer
-				*/
+/* keep this in sync with preloaded bpf,
+ * needs to be a power of 2 as we use it as a circular buffer
+ */
+#define HID_BPF_MAX_PROGS 1024
 
 #define NEXT(idx) (((idx) + 1) & (HID_BPF_MAX_PROGS - 1))
 #define PREV(idx) (((idx) - 1) & (HID_BPF_MAX_PROGS - 1))

-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] HID: bpf: fix return value of entrypoints_*__attach()
  2024-04-19 14:47 [PATCH 0/3] HID: bpf: some fixes for pre-loading HID-BPF Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 1/3] HID: bpf: fix a comment in a define Benjamin Tissoires
@ 2024-04-19 14:47 ` Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint Benjamin Tissoires
  2 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-19 14:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Benjamin Tissoires, stable

Turns out that this function returns the attached fd, so a positive
or null value.
Given that we were having no other fds before, we were receiving 0,
and the test passed.

Cc: stable@vger.kernel.org
Fixes: f5c27da4e3c8 ("HID: initial BPF implementation")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/bpf/hid_bpf_jmp_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 89496aabbe15..301ac79db241 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -521,7 +521,7 @@ void hid_bpf_free_links_and_skel(void)
 
 #define ATTACH_AND_STORE_LINK(__name) do {					\
 	err = entrypoints_bpf__##__name##__attach(skel);			\
-	if (err)								\
+	if (err < 0)								\
 		goto out;							\
 										\
 	links[idx] = bpf_link_get_from_fd(skel->links.__name##_fd);		\

-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
  2024-04-19 14:47 [PATCH 0/3] HID: bpf: some fixes for pre-loading HID-BPF Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 1/3] HID: bpf: fix a comment in a define Benjamin Tissoires
  2024-04-19 14:47 ` [PATCH 2/3] HID: bpf: fix return value of entrypoints_*__attach() Benjamin Tissoires
@ 2024-04-19 14:47 ` Benjamin Tissoires
  2024-04-23  7:10   ` Dan Carpenter
  2 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-19 14:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, Benjamin Tissoires, stable

Turns out that on some professional distributions, with things partly
backported (not sure exactly), loading this kernel bpf program might
enter a RCU task deadlock.

Given that it actually does not make sense to preload this in every
environment, we can lazy load it the first time we need it, i.e. the
first time the kfunc hid_bpf_attach_prog() is called.

Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/bpf/hid_bpf_dispatch.c  | 6 ------
 drivers/hid/bpf/hid_bpf_jmp_table.c | 7 +++++++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 10289f44d0cc..1946ad962d03 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -642,12 +642,6 @@ static int __init hid_bpf_init(void)
 		return 0;
 	}
 
-	err = hid_bpf_preload_skel();
-	if (err) {
-		pr_warn("error while preloading HID BPF dispatcher: %d", err);
-		return 0;
-	}
-
 	/* register tracing kfuncs after we are sure we can load our preloaded bpf program */
 	err = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &hid_bpf_kfunc_set);
 	if (err) {
diff --git a/drivers/hid/bpf/hid_bpf_jmp_table.c b/drivers/hid/bpf/hid_bpf_jmp_table.c
index 301ac79db241..75ce215f0ada 100644
--- a/drivers/hid/bpf/hid_bpf_jmp_table.c
+++ b/drivers/hid/bpf/hid_bpf_jmp_table.c
@@ -404,6 +404,13 @@ __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
 
 	mutex_lock(&hid_bpf_attach_lock);
 
+	if (!jmp_table.map) {
+		err = hid_bpf_preload_skel();
+		WARN_ONCE(err, "error while preloading HID BPF dispatcher: %d", err);
+		if (err)
+			goto err_unlock;
+	}
+
 	link = kzalloc(sizeof(*link), GFP_USER);
 	if (!link) {
 		err = -ENOMEM;

-- 
2.44.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
  2024-04-19 14:47 ` [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint Benjamin Tissoires
@ 2024-04-23  7:10   ` Dan Carpenter
  2024-04-24 14:17     ` Benjamin Tissoires
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2024-04-23  7:10 UTC (permalink / raw)
  To: oe-kbuild, Benjamin Tissoires, Jiri Kosina, Benjamin Tissoires
  Cc: lkp, oe-kbuild-all, linux-input, linux-kernel, stable

Hi Benjamin,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Tissoires/HID-bpf-fix-a-comment-in-a-define/20240419-225110
base:   b912cf042072e12e93faa874265b30cc0aa521b9
patch link:    https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40kernel.org
patch subject: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
config: i386-randconfig-141-20240423 (https://download.01.org/0day-ci/archive/20240423/202404231109.h2IRrMMD-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202404231109.h2IRrMMD-lkp@intel.com/

smatch warnings:
drivers/hid/bpf/hid_bpf_jmp_table.c:478 __hid_bpf_attach_prog() error: uninitialized symbol 'link'.

vim +/link +478 drivers/hid/bpf/hid_bpf_jmp_table.c

f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  396  noinline int
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  397  __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
7cdd2108903a4e3 Benjamin Tissoires 2024-01-24  398  		      int prog_fd, struct bpf_prog *prog, __u32 flags)
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  399  {
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  400  	struct bpf_link_primer link_primer;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  401  	struct hid_bpf_link *link;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  402  	struct hid_bpf_prog_entry *prog_entry;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  403  	int cnt, err = -EINVAL, prog_table_idx = -1;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  404  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  405  	mutex_lock(&hid_bpf_attach_lock);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  406  
60caa381da7dc38 Benjamin Tissoires 2024-04-19  407  	if (!jmp_table.map) {
60caa381da7dc38 Benjamin Tissoires 2024-04-19  408  		err = hid_bpf_preload_skel();
60caa381da7dc38 Benjamin Tissoires 2024-04-19  409  		WARN_ONCE(err, "error while preloading HID BPF dispatcher: %d", err);
60caa381da7dc38 Benjamin Tissoires 2024-04-19  410  		if (err)
60caa381da7dc38 Benjamin Tissoires 2024-04-19  411  			goto err_unlock;
                                                                        ^^^^^^^^^^^^^^^^
link isn't initialized.

60caa381da7dc38 Benjamin Tissoires 2024-04-19  412  	}
60caa381da7dc38 Benjamin Tissoires 2024-04-19  413  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  414  	link = kzalloc(sizeof(*link), GFP_USER);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  415  	if (!link) {
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  416  		err = -ENOMEM;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  417  		goto err_unlock;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  418  	}
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  419  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  420  	bpf_link_init(&link->link, BPF_LINK_TYPE_UNSPEC,
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  421  		      &hid_bpf_link_lops, prog);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  422  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  423  	/* do not attach too many programs to a given HID device */
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  424  	cnt = hid_bpf_program_count(hdev, NULL, prog_type);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  425  	if (cnt < 0) {
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  426  		err = cnt;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  427  		goto err_unlock;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  428  	}
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  429  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  430  	if (cnt >= hid_bpf_max_programs(prog_type)) {
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  431  		err = -E2BIG;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  432  		goto err_unlock;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  433  	}
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  434  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  435  	prog_table_idx = hid_bpf_insert_prog(prog_fd, prog);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  436  	/* if the jmp table is full, abort */
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  437  	if (prog_table_idx < 0) {
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  438  		err = prog_table_idx;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  439  		goto err_unlock;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  440  	}
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  441  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  442  	if (flags & HID_BPF_FLAG_INSERT_HEAD) {
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  443  		/* take the previous prog_entry slot */
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  444  		jmp_table.tail = PREV(jmp_table.tail);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  445  		prog_entry = &jmp_table.entries[jmp_table.tail];
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  446  	} else {
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  447  		/* take the next prog_entry slot */
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  448  		prog_entry = &jmp_table.entries[jmp_table.head];
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  449  		jmp_table.head = NEXT(jmp_table.head);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  450  	}
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  451  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  452  	/* we steal the ref here */
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  453  	prog_entry->prog = prog;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  454  	prog_entry->idx = prog_table_idx;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  455  	prog_entry->hdev = hdev;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  456  	prog_entry->type = prog_type;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  457  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  458  	/* finally store the index in the device list */
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  459  	err = hid_bpf_populate_hdev(hdev, prog_type);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  460  	if (err) {
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  461  		hid_bpf_release_prog_at(prog_table_idx);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  462  		goto err_unlock;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  463  	}
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  464  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  465  	link->hid_table_index = prog_table_idx;
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  466  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  467  	err = bpf_link_prime(&link->link, &link_primer);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  468  	if (err)
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  469  		goto err_unlock;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  470  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  471  	mutex_unlock(&hid_bpf_attach_lock);
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  472  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  473  	return bpf_link_settle(&link_primer);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  474  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  475   err_unlock:
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  476  	mutex_unlock(&hid_bpf_attach_lock);
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  477  
4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13 @478  	kfree(link);
                                                              ^^^^

f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  479  
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  480  	return err;
f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  481  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
  2024-04-23  7:10   ` Dan Carpenter
@ 2024-04-24 14:17     ` Benjamin Tissoires
  2024-04-25 12:52       ` Benjamin Tissoires
  0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-24 14:17 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Jiri Kosina, Benjamin Tissoires, lkp, oe-kbuild-all,
	linux-input, linux-kernel, stable

On Apr 23 2024, Dan Carpenter wrote:
> Hi Benjamin,
> 
> kernel test robot noticed the following build warnings:
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Tissoires/HID-bpf-fix-a-comment-in-a-define/20240419-225110
> base:   b912cf042072e12e93faa874265b30cc0aa521b9
> patch link:    https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40kernel.org
> patch subject: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
> config: i386-randconfig-141-20240423 (https://download.01.org/0day-ci/archive/20240423/202404231109.h2IRrMMD-lkp@intel.com/config)
> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202404231109.h2IRrMMD-lkp@intel.com/
> 
> smatch warnings:
> drivers/hid/bpf/hid_bpf_jmp_table.c:478 __hid_bpf_attach_prog() error: uninitialized symbol 'link'.
> 
> vim +/link +478 drivers/hid/bpf/hid_bpf_jmp_table.c
> 
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  396  noinline int
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  397  __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
> 7cdd2108903a4e3 Benjamin Tissoires 2024-01-24  398  		      int prog_fd, struct bpf_prog *prog, __u32 flags)
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  399  {
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  400  	struct bpf_link_primer link_primer;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  401  	struct hid_bpf_link *link;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  402  	struct hid_bpf_prog_entry *prog_entry;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  403  	int cnt, err = -EINVAL, prog_table_idx = -1;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  404  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  405  	mutex_lock(&hid_bpf_attach_lock);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  406  
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  407  	if (!jmp_table.map) {
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  408  		err = hid_bpf_preload_skel();
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  409  		WARN_ONCE(err, "error while preloading HID BPF dispatcher: %d", err);
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  410  		if (err)
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  411  			goto err_unlock;
>                                                                         ^^^^^^^^^^^^^^^^
> link isn't initialized.

Well spotted! Thanks
I'll send a v2 soon.

Cheers,
Benjamin

> 
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  412  	}
> 60caa381da7dc38 Benjamin Tissoires 2024-04-19  413  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  414  	link = kzalloc(sizeof(*link), GFP_USER);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  415  	if (!link) {
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  416  		err = -ENOMEM;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  417  		goto err_unlock;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  418  	}
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  419  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  420  	bpf_link_init(&link->link, BPF_LINK_TYPE_UNSPEC,
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  421  		      &hid_bpf_link_lops, prog);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  422  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  423  	/* do not attach too many programs to a given HID device */
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  424  	cnt = hid_bpf_program_count(hdev, NULL, prog_type);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  425  	if (cnt < 0) {
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  426  		err = cnt;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  427  		goto err_unlock;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  428  	}
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  429  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  430  	if (cnt >= hid_bpf_max_programs(prog_type)) {
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  431  		err = -E2BIG;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  432  		goto err_unlock;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  433  	}
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  434  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  435  	prog_table_idx = hid_bpf_insert_prog(prog_fd, prog);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  436  	/* if the jmp table is full, abort */
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  437  	if (prog_table_idx < 0) {
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  438  		err = prog_table_idx;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  439  		goto err_unlock;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  440  	}
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  441  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  442  	if (flags & HID_BPF_FLAG_INSERT_HEAD) {
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  443  		/* take the previous prog_entry slot */
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  444  		jmp_table.tail = PREV(jmp_table.tail);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  445  		prog_entry = &jmp_table.entries[jmp_table.tail];
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  446  	} else {
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  447  		/* take the next prog_entry slot */
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  448  		prog_entry = &jmp_table.entries[jmp_table.head];
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  449  		jmp_table.head = NEXT(jmp_table.head);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  450  	}
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  451  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  452  	/* we steal the ref here */
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  453  	prog_entry->prog = prog;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  454  	prog_entry->idx = prog_table_idx;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  455  	prog_entry->hdev = hdev;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  456  	prog_entry->type = prog_type;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  457  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  458  	/* finally store the index in the device list */
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  459  	err = hid_bpf_populate_hdev(hdev, prog_type);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  460  	if (err) {
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  461  		hid_bpf_release_prog_at(prog_table_idx);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  462  		goto err_unlock;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  463  	}
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  464  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  465  	link->hid_table_index = prog_table_idx;
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  466  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  467  	err = bpf_link_prime(&link->link, &link_primer);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  468  	if (err)
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  469  		goto err_unlock;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  470  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  471  	mutex_unlock(&hid_bpf_attach_lock);
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  472  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  473  	return bpf_link_settle(&link_primer);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  474  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  475   err_unlock:
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  476  	mutex_unlock(&hid_bpf_attach_lock);
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  477  
> 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13 @478  	kfree(link);
>                                                               ^^^^
> 
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  479  
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  480  	return err;
> f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  481  }
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
  2024-04-24 14:17     ` Benjamin Tissoires
@ 2024-04-25 12:52       ` Benjamin Tissoires
  0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2024-04-25 12:52 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Jiri Kosina, Benjamin Tissoires, lkp, oe-kbuild-all,
	linux-input, linux-kernel, stable

On Apr 24 2024, Benjamin Tissoires wrote:
> On Apr 23 2024, Dan Carpenter wrote:
> > Hi Benjamin,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Tissoires/HID-bpf-fix-a-comment-in-a-define/20240419-225110
> > base:   b912cf042072e12e93faa874265b30cc0aa521b9
> > patch link:    https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40kernel.org
> > patch subject: [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint
> > config: i386-randconfig-141-20240423 (https://download.01.org/0day-ci/archive/20240423/202404231109.h2IRrMMD-lkp@intel.com/config)
> > compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202404231109.h2IRrMMD-lkp@intel.com/
> > 
> > smatch warnings:
> > drivers/hid/bpf/hid_bpf_jmp_table.c:478 __hid_bpf_attach_prog() error: uninitialized symbol 'link'.
> > 
> > vim +/link +478 drivers/hid/bpf/hid_bpf_jmp_table.c
> > 
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  396  noinline int
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  397  __hid_bpf_attach_prog(struct hid_device *hdev, enum hid_bpf_prog_type prog_type,
> > 7cdd2108903a4e3 Benjamin Tissoires 2024-01-24  398  		      int prog_fd, struct bpf_prog *prog, __u32 flags)
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  399  {
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  400  	struct bpf_link_primer link_primer;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  401  	struct hid_bpf_link *link;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  402  	struct hid_bpf_prog_entry *prog_entry;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  403  	int cnt, err = -EINVAL, prog_table_idx = -1;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  404  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  405  	mutex_lock(&hid_bpf_attach_lock);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  406  
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  407  	if (!jmp_table.map) {
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  408  		err = hid_bpf_preload_skel();
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  409  		WARN_ONCE(err, "error while preloading HID BPF dispatcher: %d", err);
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  410  		if (err)
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  411  			goto err_unlock;
> >                                                                         ^^^^^^^^^^^^^^^^
> > link isn't initialized.
> 
> Well spotted! Thanks
> I'll send a v2 soon.

FWIW, this patch prevents the proper unloading of the HID-BPF programs.
So not sure we'll want it as such just now.

Cheers,
Benjamin

> > 
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  412  	}
> > 60caa381da7dc38 Benjamin Tissoires 2024-04-19  413  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  414  	link = kzalloc(sizeof(*link), GFP_USER);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  415  	if (!link) {
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  416  		err = -ENOMEM;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  417  		goto err_unlock;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  418  	}
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  419  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  420  	bpf_link_init(&link->link, BPF_LINK_TYPE_UNSPEC,
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  421  		      &hid_bpf_link_lops, prog);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  422  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  423  	/* do not attach too many programs to a given HID device */
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  424  	cnt = hid_bpf_program_count(hdev, NULL, prog_type);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  425  	if (cnt < 0) {
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  426  		err = cnt;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  427  		goto err_unlock;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  428  	}
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  429  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  430  	if (cnt >= hid_bpf_max_programs(prog_type)) {
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  431  		err = -E2BIG;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  432  		goto err_unlock;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  433  	}
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  434  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  435  	prog_table_idx = hid_bpf_insert_prog(prog_fd, prog);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  436  	/* if the jmp table is full, abort */
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  437  	if (prog_table_idx < 0) {
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  438  		err = prog_table_idx;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  439  		goto err_unlock;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  440  	}
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  441  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  442  	if (flags & HID_BPF_FLAG_INSERT_HEAD) {
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  443  		/* take the previous prog_entry slot */
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  444  		jmp_table.tail = PREV(jmp_table.tail);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  445  		prog_entry = &jmp_table.entries[jmp_table.tail];
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  446  	} else {
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  447  		/* take the next prog_entry slot */
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  448  		prog_entry = &jmp_table.entries[jmp_table.head];
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  449  		jmp_table.head = NEXT(jmp_table.head);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  450  	}
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  451  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  452  	/* we steal the ref here */
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  453  	prog_entry->prog = prog;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  454  	prog_entry->idx = prog_table_idx;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  455  	prog_entry->hdev = hdev;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  456  	prog_entry->type = prog_type;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  457  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  458  	/* finally store the index in the device list */
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  459  	err = hid_bpf_populate_hdev(hdev, prog_type);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  460  	if (err) {
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  461  		hid_bpf_release_prog_at(prog_table_idx);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  462  		goto err_unlock;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  463  	}
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  464  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  465  	link->hid_table_index = prog_table_idx;
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  466  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  467  	err = bpf_link_prime(&link->link, &link_primer);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  468  	if (err)
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  469  		goto err_unlock;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  470  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  471  	mutex_unlock(&hid_bpf_attach_lock);
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  472  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  473  	return bpf_link_settle(&link_primer);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  474  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  475   err_unlock:
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  476  	mutex_unlock(&hid_bpf_attach_lock);
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13  477  
> > 4b9a3f49f02bf68 Benjamin Tissoires 2023-01-13 @478  	kfree(link);
> >                                                               ^^^^
> > 
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  479  
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  480  	return err;
> > f5c27da4e3c8a2e Benjamin Tissoires 2022-11-03  481  }
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
> > 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-25 12:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 14:47 [PATCH 0/3] HID: bpf: some fixes for pre-loading HID-BPF Benjamin Tissoires
2024-04-19 14:47 ` [PATCH 1/3] HID: bpf: fix a comment in a define Benjamin Tissoires
2024-04-19 14:47 ` [PATCH 2/3] HID: bpf: fix return value of entrypoints_*__attach() Benjamin Tissoires
2024-04-19 14:47 ` [PATCH 3/3] HID: bpf: lazy load the hid_tail_call entrypoint Benjamin Tissoires
2024-04-23  7:10   ` Dan Carpenter
2024-04-24 14:17     ` Benjamin Tissoires
2024-04-25 12:52       ` Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).