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,
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))
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); \
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;
Hi Benjamin,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Tissoires/HID-bpf-fi... base: b912cf042072e12e93faa874265b30cc0aa521b9 patch link: https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40ker... 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@i...) 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 }
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-fi... base: b912cf042072e12e93faa874265b30cc0aa521b9 patch link: https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40ker... 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@i...) 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
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-fi... base: b912cf042072e12e93faa874265b30cc0aa521b9 patch link: https://lore.kernel.org/r/20240419-hid_bpf_lazy_skel-v1-3-9210bcd4b61c%40ker... 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@i...) 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
linux-stable-mirror@lists.linaro.org
-
Benjamin Tissoires -
Dan Carpenter