Please check the goto on line 2663. Is an unlock needed here? julia
---------- Forwarded message ----------
Date: Mon, 12 Apr 2021 01:28:54 +0800
From: kernel test robot <l...@intel.com>
To: kbu...@lists.01.org
Cc: l...@intel.com, Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCHv3 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing
and lsm programs
CC: kbuild-...@lists.01.org
In-Reply-To: <20210411130010.1337650-2-jo...@kernel.org>
References: <20210411130010.1337650-2-jo...@kernel.org>
TO: Jiri Olsa <jo...@kernel.org>
TO: Alexei Starovoitov <a...@kernel.org>
TO: Daniel Borkmann <dan...@iogearbox.net>
TO: Andrii Nakryiko <andr...@fb.com>
CC: "Toke Høiland-Jørgensen" <t...@redhat.com>
CC: netdev@vger.kernel.org
CC: b...@vger.kernel.org
CC: Martin KaFai Lau <ka...@fb.com>
CC: Song Liu <songliubrav...@fb.com>
CC: Yonghong Song <y...@fb.com>
CC: John Fastabend <john.fastab...@gmail.com>
Hi Jiri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url:
https://github.com/0day-ci/linux/commits/Jiri-Olsa/bpf-Tracing-and-lsm-programs-re-attach/20210411-210314
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
:::::: branch date: 4 hours ago
:::::: commit date: 4 hours ago
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>
Reported-by: Julia Lawall <julia.law...@lip6.fr>
cocci warnings: (new ones prefixed by >>)
>> kernel/bpf/syscall.c:2738:1-7: preceding lock on line 2633
vim +2738 kernel/bpf/syscall.c
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2564
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2565 static int
bpf_tracing_prog_attach(struct bpf_prog *prog,
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2566
int tgt_prog_fd,
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2567
u32 btf_id)
fec56f5890d93f Alexei Starovoitov 2019-11-14 2568 {
a3b80e1078943d Andrii Nakryiko 2020-04-28 2569 struct
bpf_link_primer link_primer;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2570 struct bpf_prog
*tgt_prog = NULL;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2571 struct
bpf_trampoline *tr = NULL;
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2572 struct
bpf_tracing_link *link;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2573 u64 key = 0;
a3b80e1078943d Andrii Nakryiko 2020-04-28 2574 int err;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2575
9e4e01dfd3254c KP Singh 2020-03-29 2576 switch
(prog->type) {
9e4e01dfd3254c KP Singh 2020-03-29 2577 case
BPF_PROG_TYPE_TRACING:
fec56f5890d93f Alexei Starovoitov 2019-11-14 2578 if
(prog->expected_attach_type != BPF_TRACE_FENTRY &&
be8704ff07d237 Alexei Starovoitov 2020-01-20 2579
prog->expected_attach_type != BPF_TRACE_FEXIT &&
9e4e01dfd3254c KP Singh 2020-03-29 2580
prog->expected_attach_type != BPF_MODIFY_RETURN) {
9e4e01dfd3254c KP Singh 2020-03-29 2581
err = -EINVAL;
9e4e01dfd3254c KP Singh 2020-03-29 2582
goto out_put_prog;
9e4e01dfd3254c KP Singh 2020-03-29 2583 }
9e4e01dfd3254c KP Singh 2020-03-29 2584 break;
9e4e01dfd3254c KP Singh 2020-03-29 2585 case
BPF_PROG_TYPE_EXT:
9e4e01dfd3254c KP Singh 2020-03-29 2586 if
(prog->expected_attach_type != 0) {
9e4e01dfd3254c KP Singh 2020-03-29 2587
err = -EINVAL;
9e4e01dfd3254c KP Singh 2020-03-29 2588
goto out_put_prog;
9e4e01dfd3254c KP Singh 2020-03-29 2589 }
9e4e01dfd3254c KP Singh 2020-03-29 2590 break;
9e4e01dfd3254c KP Singh 2020-03-29 2591 case
BPF_PROG_TYPE_LSM:
9e4e01dfd3254c KP Singh 2020-03-29 2592 if
(prog->expected_attach_type != BPF_LSM_MAC) {
9e4e01dfd3254c KP Singh 2020-03-29 2593
err = -EINVAL;
9e4e01dfd3254c KP Singh 2020-03-29 2594
goto out_put_prog;
9e4e01dfd3254c KP Singh 2020-03-29 2595 }
9e4e01dfd3254c KP Singh 2020-03-29 2596 break;
9e4e01dfd3254c KP Singh 2020-03-29 2597 default:
fec56f5890d93f Alexei Starovoitov 2019-11-14 2598 err =
-EINVAL;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2599 goto
out_put_prog;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2600 }
fec56f5890d93f Alexei Starovoitov 2019-11-14 2601
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2602 if
(!!tgt_prog_fd != !!btf_id) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2603 err =
-EINVAL;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2604 goto
out_put_prog;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2605 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2606
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2607 if
(tgt_prog_fd) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2608 /* For
now we only allow new targets for BPF_PROG_TYPE_EXT */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2609 if
(prog->type != BPF_PROG_TYPE_EXT) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2610
err = -EINVAL;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2611
goto out_put_prog;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2612 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2613
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2614
tgt_prog = bpf_prog_get(tgt_prog_fd);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2615 if
(IS_ERR(tgt_prog)) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2616
err = PTR_ERR(tgt_prog);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2617
tgt_prog = NULL;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2618
goto out_put_prog;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2619 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2620
22dc4a0f5ed11b Andrii Nakryiko 2020-12-03 2621 key =
bpf_trampoline_compute_key(tgt_prog, NULL, btf_id);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2622 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2623
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2624 link =
kzalloc(sizeof(*link), GFP_USER);
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2625 if (!link) {
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2626 err =
-ENOMEM;
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2627 goto
out_put_prog;
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2628 }
f2e10bff16a0fd Andrii Nakryiko 2020-04-28 2629
bpf_link_init(&link->link, BPF_LINK_TYPE_TRACING,
f2e10bff16a0fd Andrii Nakryiko 2020-04-28 2630
&bpf_tracing_link_lops, prog);
f2e10bff16a0fd Andrii Nakryiko 2020-04-28 2631
link->attach_type = prog->expected_attach_type;
70ed506c3bbcfa Andrii Nakryiko 2020-03-02 2632
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 @2633
mutex_lock(&prog->aux->dst_mutex);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2634
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2635 /* There are a
few possible cases here:
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2636 *
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2637 * - if
prog->aux->dst_trampoline is set, the program was just loaded
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2638 * and not
yet attached to anything, so we can use the values stored
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2639 * in
prog->aux
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2640 *
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2641 * - if
prog->aux->dst_trampoline is NULL, the program has already been
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2642 * attached
to a target and its initial target was cleared (below)
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2643 *
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2644 * - if
tgt_prog != NULL, the caller specified tgt_prog_fd +
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2645 *
target_btf_id using the link_create API.
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2646 *
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2647 * - if
tgt_prog == NULL when this function was called using the old
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2648 *
raw_tracepoint_open API, and we need a target from prog->aux
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2649 *
fc909cd5516914 Jiri Olsa 2021-04-11 2650 * - if
prog->aux->dst_trampoline and tgt_prog is NULL, the program
fc909cd5516914 Jiri Olsa 2021-04-11 2651 * was
detached and is going for re-attachment.
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2652 */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2653 if
(!prog->aux->dst_trampoline && !tgt_prog) {
fc909cd5516914 Jiri Olsa 2021-04-11 2654 /*
fc909cd5516914 Jiri Olsa 2021-04-11 2655 *
Allow re-attach for TRACING and LSM programs. If it's
fc909cd5516914 Jiri Olsa 2021-04-11 2656 *
currently linked, bpf_trampoline_link_prog will fail.
fc909cd5516914 Jiri Olsa 2021-04-11 2657 * EXT
programs need to specify tgt_prog_fd, so they
fc909cd5516914 Jiri Olsa 2021-04-11 2658 *
re-attach in separate code path.
fc909cd5516914 Jiri Olsa 2021-04-11 2659 */
fc909cd5516914 Jiri Olsa 2021-04-11 2660 if
(prog->type != BPF_PROG_TYPE_TRACING &&
fc909cd5516914 Jiri Olsa 2021-04-11 2661
prog->type != BPF_PROG_TYPE_LSM) {
fc909cd5516914 Jiri Olsa 2021-04-11 2662
err = -EINVAL;
fc909cd5516914 Jiri Olsa 2021-04-11 2663
goto out_put_prog;
fc909cd5516914 Jiri Olsa 2021-04-11 2664 }
fc909cd5516914 Jiri Olsa 2021-04-11 2665 btf_id
= prog->aux->attach_btf_id;
fc909cd5516914 Jiri Olsa 2021-04-11 2666 key =
bpf_trampoline_compute_key(NULL, prog->aux->attach_btf, btf_id);
babf3164095b06 Andrii Nakryiko 2020-03-09 2667 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2668
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2669 if
(!prog->aux->dst_trampoline ||
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2670 (key && key
!= prog->aux->dst_trampoline->key)) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2671 /* If
there is no saved target, or the specified target is
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2672 *
different from the destination specified at load time, we
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2673 * need
a new trampoline and a check for compatibility
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2674 */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2675 struct
bpf_attach_target_info tgt_info = {};
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2676
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2677 err =
bpf_check_attach_target(NULL, prog, tgt_prog, btf_id,
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2678
&tgt_info);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2679 if (err)
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2680
goto out_unlock;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2681
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2682 tr =
bpf_trampoline_get(key, &tgt_info);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2683 if
(!tr) {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2684
err = -ENOMEM;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2685
goto out_unlock;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2686 }
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2687 } else {
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2688 /* The
caller didn't specify a target, or the target was the
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2689 * same
as the destination supplied during program load. This
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2690 *
means we can reuse the trampoline and reference from program
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2691 * load
time, and there is no need to allocate a new one. This
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2692 * can
only happen once for any program, as the saved values in
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2693 *
prog->aux are cleared below.
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2694 */
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2695 tr =
prog->aux->dst_trampoline;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2696
tgt_prog = prog->aux->dst_prog;
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2697 }
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2698
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2699 err =
bpf_link_prime(&link->link, &link_primer);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2700 if (err)
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2701 goto
out_unlock;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2702
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2703 err =
bpf_trampoline_link_prog(prog, tr);
babf3164095b06 Andrii Nakryiko 2020-03-09 2704 if (err) {
a3b80e1078943d Andrii Nakryiko 2020-04-28 2705
bpf_link_cleanup(&link_primer);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2706 link =
NULL;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2707 goto
out_unlock;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2708 }
babf3164095b06 Andrii Nakryiko 2020-03-09 2709
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2710 link->tgt_prog
= tgt_prog;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2711
link->trampoline = tr;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2712
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2713 /* Always clear
the trampoline and target prog from prog->aux to make
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2714 * sure the
original attach destination is not kept alive after a
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2715 * program is
(re-)attached to another target.
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2716 */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2717 if
(prog->aux->dst_prog &&
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2718
(tgt_prog_fd || tr != prog->aux->dst_trampoline))
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2719 /* got
extra prog ref from syscall, or attaching to different prog */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2720
bpf_prog_put(prog->aux->dst_prog);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2721 if
(prog->aux->dst_trampoline && tr != prog->aux->dst_trampoline)
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2722 /* we
allocated a new trampoline, so free the old one */
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2723
bpf_trampoline_put(prog->aux->dst_trampoline);
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2724
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2725
prog->aux->dst_prog = NULL;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2726
prog->aux->dst_trampoline = NULL;
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2727
mutex_unlock(&prog->aux->dst_mutex);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2728
a3b80e1078943d Andrii Nakryiko 2020-04-28 2729 return
bpf_link_settle(&link_primer);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2730 out_unlock:
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2731 if (tr && tr !=
prog->aux->dst_trampoline)
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2732
bpf_trampoline_put(tr);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2733
mutex_unlock(&prog->aux->dst_mutex);
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 2734 kfree(link);
fec56f5890d93f Alexei Starovoitov 2019-11-14 2735 out_put_prog:
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2736 if (tgt_prog_fd
&& tgt_prog)
4a1e7c0c63e02d Toke Høiland-Jørgensen 2020-09-29 2737
bpf_prog_put(tgt_prog);
fec56f5890d93f Alexei Starovoitov 2019-11-14 @2738 return err;
fec56f5890d93f Alexei Starovoitov 2019-11-14 2739 }
fec56f5890d93f Alexei Starovoitov 2019-11-14 2740
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
