On Sun, Apr 11, 2021 at 07:54:34PM +0200, Julia Lawall wrote: > Please check the goto on line 2663. Is an unlock needed here?
oops, yes, it's missing unlock.. I'll send new version thanks, jirka > > 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
