From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5C804C48BC3 for ; Tue, 20 Feb 2024 17:18:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kXUDocPeFdC+Ru1E5XHNxw3GnrTSkDRh4m8HfT2mcsI=; b=vmWa5PWAx/rQ/X wL9wVYA1v6bzl9XvPC0oii/vfpzr7hBpsBnV7z4h0bok+AdVJkydhgnzxNnHzshEs2UIpWeY794Yn U2ATHeR0JBE+RMCal3P0B8lm1XS4Mca1oFv4jtkg8l5UiMrpiDHz+ToIj5t0OK7r0LE10lE6DahST El9HL3DkkOD/t1Zu2FN4JozOWiGC+PxzQB8Ixp9Drh9Kzer+qkQeWL0fBW+lHydguLbutJqAjQOMo +3+ou8/A4QEalMIBagqHwXgB55arHEObLBglH8TACK+SUSWcJ6weuAfnanrL693MUG1MkhBadjT71 NuKv2FWoxB0KjZpMeYyA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcTlO-0000000Fbem-3HIP; Tue, 20 Feb 2024 17:18:38 +0000 Received: from mail-ej1-x62d.google.com ([2a00:1450:4864:20::62d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcTlM-0000000FbdX-0cVu for linux-arm-kernel@lists.infradead.org; Tue, 20 Feb 2024 17:18:37 +0000 Received: by mail-ej1-x62d.google.com with SMTP id a640c23a62f3a-a3e75e30d36so449123966b.1 for ; Tue, 20 Feb 2024 09:18:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708449513; x=1709054313; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:from:to:cc:subject:date:message-id:reply-to; bh=dmxS6iktBtidz6yGIwqms+MtYfTcC8Gwrx4Li+2LctM=; b=Ogu266ftASRHFR6uXERpTeaRv0k3IY2qZ0Zn3RgkP2N4K20X5MvDFXnirLMKF6DOxq fhwo9Rxu6SmB+IQpCw1R5svbiaCXFPpmamo3gW1KAmsxYD/mecAZgtKKvK8zubZwz4mN igHGE75xN/r79q0drcbgIhdKI6YTA9v+eJUbdG0Lx0z5oPuH1LkRrQGjCxF2NDom12p6 XzIQWSptc8Haj2wTsQnuipmn+uBpxZKYQ0vQiPPhFlBz7ot3z4E6xI2X+fdMLnF6jaFW /Y2vCLPQq6dbQlhywNT4VGxz3heYHgs+cFipkJu+FS5+c9TjDvxP6anli+A/GtBjkqUD QkFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708449513; x=1709054313; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=dmxS6iktBtidz6yGIwqms+MtYfTcC8Gwrx4Li+2LctM=; b=L5EzRidYBCvXrxCvHeg+12OGMlQihfT5cpitIjlYp8C8D7dV2aSVDttqtcK9Ykah56 ssRsClUl54j+/7r6Bt6b/ZCGP2HS1M/koAtNsOUAmHb+4z+XK4znl3jFHUjwYBc4Qbg4 AvMp0o2b5r/AjTTZbdS5sJfoYQDo6EJ+Gew+AjuuVYKdLZlOoYQIg0Rc0TtEflJJJJ8J CU0fUEme5YL56DJqGooCqUeMq+cz+G73PipVW41qdLHwP8BmV1x48xGPXR6sHvA6ZZ22 7OjjthmHBLNBMgg1TITYI/XBUjFYYZ9fRanE/8v+dCXkhruGvdoXPEdQ+oKwDQ1fKoWV X3ZQ== X-Forwarded-Encrypted: i=1; AJvYcCUWR5ZqqBKV4vFlPuElSsv1nL9IiMolFI0cLqtxfdYiZerwIm8R9rZvx6vdohipfDKBqH7t7pX2hE/e95b30ziAGuoiwkM4Hd8vzg8RF4B+PQCK3NA= X-Gm-Message-State: AOJu0YypZboHZtdna650nbkzsMAo9GSwygd8hZXemGgY5ttdAFo0u/Sb PVnZ9vvNM+NaQzGuNS6yPV6Fw8GfkNiGfsUPkMTvGJ76XYSraI9v X-Google-Smtp-Source: AGHT+IFcO8MiaoD6Cte2ItgLhV+w9LAMsoeBHZyEn5ZrO/x7ZBnibRjWEqOEGIuYMtJYCpOsxEMCFg== X-Received: by 2002:a17:906:28d1:b0:a3e:c77a:8100 with SMTP id p17-20020a17090628d100b00a3ec77a8100mr4485554ejd.17.1708449513035; Tue, 20 Feb 2024 09:18:33 -0800 (PST) Received: from krava ([83.240.60.70]) by smtp.gmail.com with ESMTPSA id g1-20020a170906c18100b00a3e278c4a3fsm3668349ejz.53.2024.02.20.09.18.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Feb 2024 09:18:32 -0800 (PST) From: Jiri Olsa X-Google-Original-From: Jiri Olsa Date: Tue, 20 Feb 2024 18:18:30 +0100 To: Menglong Dong Cc: andrii@kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, zhoufeng.zf@bytedance.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH bpf-next 2/5] bpf: tracing: support to attach program to multi hooks Message-ID: References: <20240220035105.34626-1-dongmenglong.8@bytedance.com> <20240220035105.34626-3-dongmenglong.8@bytedance.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240220035105.34626-3-dongmenglong.8@bytedance.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240220_091836_212406_7E28E9BA X-CRM114-Status: GOOD ( 32.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Feb 20, 2024 at 11:51:02AM +0800, Menglong Dong wrote: SNIP > @@ -3228,7 +3260,9 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > struct bpf_link_primer link_primer; > struct bpf_prog *tgt_prog = NULL; > struct bpf_trampoline *tr = NULL; > + struct btf *attach_btf = NULL; > struct bpf_tracing_link *link; > + struct module *mod = NULL; > u64 key = 0; > int err; > > @@ -3258,31 +3292,50 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > goto out_put_prog; > } > > - if (!!tgt_prog_fd != !!btf_id) { > - err = -EINVAL; > - goto out_put_prog; > - } > - > if (tgt_prog_fd) { > - /* > - * For now we only allow new targets for BPF_PROG_TYPE_EXT. If this > - * part would be changed to implement the same for > - * BPF_PROG_TYPE_TRACING, do not forget to update the way how > - * attach_tracing_prog flag is set. > - */ > - if (prog->type != BPF_PROG_TYPE_EXT) { > + if (!btf_id) { > err = -EINVAL; > goto out_put_prog; > } > - > tgt_prog = bpf_prog_get(tgt_prog_fd); > if (IS_ERR(tgt_prog)) { > - err = PTR_ERR(tgt_prog); > tgt_prog = NULL; > - goto out_put_prog; > + /* tgt_prog_fd is the fd of the kernel module BTF */ > + attach_btf = btf_get_by_fd(tgt_prog_fd); I think we should pass the btf_fd through attr, like add link_create.tracing_btf_fd instead, this seems confusing > + if (IS_ERR(attach_btf)) { > + attach_btf = NULL; > + err = -EINVAL; > + goto out_put_prog; > + } > + if (!btf_is_kernel(attach_btf)) { > + btf_put(attach_btf); > + err = -EOPNOTSUPP; > + goto out_put_prog; > + } > + } else if (prog->type == BPF_PROG_TYPE_TRACING && > + tgt_prog->type == BPF_PROG_TYPE_TRACING) { > + prog->aux->attach_tracing_prog = true; > } could you please add comment on why this check is in here? > - > - key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id); > + key = bpf_trampoline_compute_key(tgt_prog, attach_btf, > + btf_id); > + } else if (btf_id) { > + attach_btf = bpf_get_btf_vmlinux(); > + if (IS_ERR(attach_btf)) { > + attach_btf = NULL; > + err = PTR_ERR(attach_btf); > + goto out_unlock; > + } > + if (!attach_btf) { > + err = -EINVAL; > + goto out_unlock; > + } > + btf_get(attach_btf); > + key = bpf_trampoline_compute_key(NULL, attach_btf, btf_id); > + } else { > + attach_btf = prog->aux->attach_btf; > + /* get the reference of the btf for bpf link */ > + if (attach_btf) > + btf_get(attach_btf); > } > > link = kzalloc(sizeof(*link), GFP_USER); > @@ -3319,7 +3372,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > * are NULL, then program was already attached and user did not provide > * tgt_prog_fd so we have no way to find out or create trampoline > */ > - if (!prog->aux->dst_trampoline && !tgt_prog) { > + if (!prog->aux->dst_trampoline && !tgt_prog && !btf_id) { > /* > * Allow re-attach for TRACING and LSM programs. If it's > * currently linked, bpf_trampoline_link_prog will fail. > @@ -3346,17 +3399,27 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > * different from the destination specified at load time, we > * need a new trampoline and a check for compatibility > */ > + struct btf *origin_btf = prog->aux->attach_btf; > struct bpf_attach_target_info tgt_info = {}; > > + /* use the new attach_btf to check the target */ > + prog->aux->attach_btf = attach_btf; > err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id, > &tgt_info); > + prog->aux->attach_btf = origin_btf; could we pass the attach_btf as argument then? jirka > if (err) > goto out_unlock; > > - if (tgt_info.tgt_mod) { > - module_put(prog->aux->mod); > - prog->aux->mod = tgt_info.tgt_mod; > - } > + mod = tgt_info.tgt_mod; > + /* the new target and the previous target are in the same > + * module, release the reference once. > + */ > + if (mod && mod == prog->aux->mod) > + module_put(mod); > + err = bpf_tracing_check_multi(prog, tgt_prog, attach_btf, > + tgt_info.tgt_type); > + if (err) > + goto out_unlock; > > tr = bpf_trampoline_get(key, &tgt_info); > if (!tr) { > @@ -3373,6 +3436,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > */ > tr = prog->aux->dst_trampoline; > tgt_prog = prog->aux->dst_prog; > + mod = prog->aux->mod; > } > > err = bpf_link_prime(&link->link.link, &link_primer); > @@ -3388,6 +3452,8 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > > link->tgt_prog = tgt_prog; > link->trampoline = tr; > + link->attach_btf = attach_btf; > + link->mod = mod; > > /* Always clear the trampoline and target prog from prog->aux to make > * sure the original attach destination is not kept alive after a > @@ -3400,20 +3466,27 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > if (prog->aux->dst_trampoline && tr != prog->aux->dst_trampoline) > /* we allocated a new trampoline, so free the old one */ > bpf_trampoline_put(prog->aux->dst_trampoline); > + if (prog->aux->mod && mod != prog->aux->mod) > + /* the mod in prog is not used anywhere, move it to link */ > + module_put(prog->aux->mod); > > prog->aux->dst_prog = NULL; > prog->aux->dst_trampoline = NULL; > + prog->aux->mod = NULL; > mutex_unlock(&prog->aux->dst_mutex); > > return bpf_link_settle(&link_primer); > out_unlock: > if (tr && tr != prog->aux->dst_trampoline) > bpf_trampoline_put(tr); > + if (mod && mod != prog->aux->mod) > + module_put(mod); > mutex_unlock(&prog->aux->dst_mutex); > kfree(link); > out_put_prog: > if (tgt_prog_fd && tgt_prog) > bpf_prog_put(tgt_prog); > + btf_put(attach_btf); > return err; > } > > -- > 2.39.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel