From patchwork Tue May 7 00:13:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656088 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AEC21652 for ; Tue, 7 May 2024 00:13:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040822; cv=none; b=aF6M0MAGOVhtgPUFpZSLiKY74KI/NMc110/DvbFOF0wehrL7u2hQab6XI/pO9s9NB5s1LuqBfWVsv3nJZbt8O6kyq//AYjrKO5Y3ZVi4//5MVQKagqBEvJmZ+bjBoTMMKTD/AvuAUNOAOeunZTOR+qiz+DSPiBPWQpxW79zincQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040822; c=relaxed/simple; bh=VnZrZBbVFpaYeB7xyMBrp4QjGtWRcvFbZGgkUzR4TGY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=LTO3SCUGIxQ8NzIN23yd8siK48MWhWPCuwbrQ43IFavrzpZberK7sr/uJAC3O1VJ5WUp+AVgjYea+iQkiKb86/chlccS3jA0V/syHjVngov8aqDOj55O08VxIJ1lOT/D33leg6BdcNoAAKd9VVuTxv5mv52C9Efcnd4fveR0xaQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=klWmrLZv; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="klWmrLZv" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 62889C116B1; Tue, 7 May 2024 00:13:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040822; bh=VnZrZBbVFpaYeB7xyMBrp4QjGtWRcvFbZGgkUzR4TGY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=klWmrLZv9d/hG22Ng7b4llvkE2RpufXK16jFt9EhbryEd7TI12BWw/X8KXdUyO0J5 a4A7coz54WhUyWcj4P+0n+zU30SFS61FlmPDqSkyZdpRh3eZK1m6+48jF2BUSygmFT SG+6WH3/eQ1zTZsSehQ0/HeXFqVzR/NVIeVtVPDpogm65ZCGHAwdlCku+Sx1Uq9fI1 NbaWgZl/mlF9s2u+Mm2yM4KSZ91POV4k6GYW32Gtaz1yvC6A9QaE1Xuy8PeJ/0Vzwm BL/0MHkF7EgpUvBzjOYPkSMsZQd+ic2NNIdzOp7y/hD/tb0Nsre/NAcGvvXfexBuNA 2OY9wCJlbI3Gw== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 1/7] libbpf: remove unnecessary struct_ops prog validity check Date: Mon, 6 May 2024 17:13:29 -0700 Message-ID: <20240507001335.1445325-2-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net libbpf ensures that BPF program references set in map->st_ops->progs[i] during open phase are always valid STRUCT_OPS programs. This is done in bpf_object__collect_st_ops_relos(). So there is no need to double-check that in bpf_map__init_kern_struct_ops(). Simplify the code by removing unnecessary check. Also, we avoid using local prog variable to keep code similar to the upcoming fix, which adds similar logic in another part of bpf_map__init_kern_struct_ops(). Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/libbpf.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index a3566a456bc8..7d77a1b158ce 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1152,22 +1152,15 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) return -ENOTSUP; } - prog = st_ops->progs[i]; - if (prog) { + if (st_ops->progs[i]) { /* If we had declaratively set struct_ops callback, we need to - * first validate that it's actually a struct_ops program. - * And then force its autoload to false, because it doesn't have + * force its autoload to false, because it doesn't have * a chance of succeeding from POV of the current struct_ops map. * If this program is still referenced somewhere else, though, * then bpf_object_adjust_struct_ops_autoload() will update its * autoload accordingly. */ - if (!is_valid_st_ops_program(obj, prog)) { - pr_warn("struct_ops init_kern %s: member %s is declaratively assigned a non-struct_ops program\n", - map->name, mname); - return -EINVAL; - } - prog->autoload = false; + st_ops->progs[i]->autoload = false; st_ops->progs[i] = NULL; } From patchwork Tue May 7 00:13:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656089 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6CCB310E6 for ; Tue, 7 May 2024 00:13:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040826; cv=none; b=Oc9RnaH6KhLMy8ox/7KcE/bofQ8nE/xHbuD2pv7PV+VtOildxELKYdqAX/r3NOC7P+mpWzkA3ODagtWgZniH6U2R+CVdYQfjLcjXsWpScxyKRE8BjeN8Y2RstKuZvhnhUbf6sx9gI7c6wq7m0br9cze3OBZ9s+NgvVXC6dtFM5E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040826; c=relaxed/simple; bh=Aw6vFEBFJ2UDRus8piXKw/61yZeVfzYsvvDvjrhBqVs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CrFlfrggMxPHsnXSgHBb6+72f4WD1CXpA/lQEFTHXNX2I1/wnIj3iZtQvMF1DtIwq6vWDUFVLmuZgnChW3eod/dqknzVvWSheTKq+CDsTRIuWhXDp1UPuR1F8ssLZovPbV6MsP1iETbvmH240a8qqHyGELnvzcY8kTyZ6dnDEI8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lADgsd8D; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lADgsd8D" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2E0CC116B1; Tue, 7 May 2024 00:13:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040825; bh=Aw6vFEBFJ2UDRus8piXKw/61yZeVfzYsvvDvjrhBqVs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lADgsd8DNDH7vneFKWnoReI9TAkdkE1Pfb+/SNyFCGpVvUEdAyGHC6TDGzb0rZX9Z RjVp3a5GX1wmHVSbID7oMCamgcOwAzhxhI+Dd3VDwH6rnlngAk8qN6g24aDNf2Zbyb /F/W0Y2e+hnjdpMWvYM1D3DSpqbJE79/b/mgC4one5h50FCYss8r0L6ud91RhxCrj2 ytP9kTA30W5KvNFJyUgJufU1f5FKcgdC4xcp49PrRpYcTDrOLiAQTkdxjRFrz+nMUw LZNgd1fj8Dj/nOZM0sN8QHagT9ijycSwu6MXKndTd/rbvGKlESpPkYFDGBw01/24aB qhN3/6AVq53qg== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 2/7] libbpf: handle yet another corner case of nulling out struct_ops program Date: Mon, 6 May 2024 17:13:30 -0700 Message-ID: <20240507001335.1445325-3-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net There is yet another corner case where user can set STRUCT_OPS program reference in STRUCT_OPS map to NULL, but libbpf will fail to disable autoload for such BPF program. This time it's the case of "new" kernel which has type information about callback field, but user explicitly nulled-out program reference from user-space after opening BPF object. Fix, hopefully, the last remaining unhandled case. Fixes: 0737df6de946 ("libbpf: better fix for handling nulled-out struct_ops program") Fixes: f973fccd43d3 ("libbpf: handle nulled-out program in struct_ops correctly") Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/libbpf.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 7d77a1b158ce..04de4fb81785 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1193,11 +1193,19 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) } if (btf_is_ptr(mtype)) { - /* Update the value from the shadow type */ prog = *(void **)mdata; + /* just like for !kern_member case above, reset declaratively + * set (at compile time) program's autload to false, + * if user replaced it with another program or NULL + */ + if (st_ops->progs[i] && st_ops->progs[i] != prog) + st_ops->progs[i]->autoload = false; + + /* Update the value from the shadow type */ st_ops->progs[i] = prog; if (!prog) continue; + if (!is_valid_st_ops_program(obj, prog)) { pr_warn("struct_ops init_kern %s: member %s is not a struct_ops program\n", map->name, mname); From patchwork Tue May 7 00:13:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656090 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 90E0710E6 for ; Tue, 7 May 2024 00:13:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040829; cv=none; b=eLogyBb/qqhVIZ0GuTwjqZdqmOPNo7lXkva63mxn1hgWUOeCAywSTLkqqL+MIBXfugZgYzW41XnF72hrcB9rl+/JKIm9Zqhcy0T7kli1L6GgVEVFwsX6TP5XSDgsjg2jkinzTzf14i7bHQ/GTb7q81thGfyw2B6pboXJbjbC6ZU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040829; c=relaxed/simple; bh=vXSXvlNAbkNh+7+KL0k+gtLsik1yPo/wXZKumA436Ag=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=SOqL6BFrTk29V1Dw0MqVcMfguYi+xp9m0bmzeQ581VV3oUV+gkXCimyT28ecr0nhH72gpzDB9Ev4Ih0H4O1o4EiRizjfXlqaQZ5bS0+3UTnioogEwKU9U+xYGqZSVgz0Xljnd/rvuD8ztpdAXu0c0rLgootm6P1yvqVh1qi9SMw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=VFwKoIVM; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="VFwKoIVM" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E59D3C116B1; Tue, 7 May 2024 00:13:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040829; bh=vXSXvlNAbkNh+7+KL0k+gtLsik1yPo/wXZKumA436Ag=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VFwKoIVMfRJK3O0Cfm9SVrhRTi24i/uJ/DFpdRIc76N6zXwo2lmfozlkMyM7EvRD8 /3JrRHB9o/FwelZvpM482YwMYOGRYKsF/yUXuZR+X4VFysyZtpx59eFC8hGK5EFtG1 2HpD6uIOlC+fPiU2yJTukcG5YbRB9ZD36IlQaxVh32gaKjJwril9OpfV/yBVMXtBs8 S70Dcd7hDDVoN1vr7Ag17cWLri3iFxydkO6gwfX/rCFVj7xLY+shHc7tpyhcGWxV6h 7hmj91EHsdwnyolz4ibHp5BO8d6NrsEWi0xC8/wXLofie6Mue06BfrPNtAeNHpDmKz ct28RITvW8xAw== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 3/7] selftests/bpf: add another struct_ops callback use case test Date: Mon, 6 May 2024 17:13:31 -0700 Message-ID: <20240507001335.1445325-4-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Add a test which tests the case that was just fixed. Kernel has full type information about callback, but user explicitly nulls out the reference to declaratively set BPF program reference. Signed-off-by: Andrii Nakryiko --- .../bpf/prog_tests/test_struct_ops_module.c | 27 +++++++++++++++++++ .../bpf/progs/struct_ops_nulled_out_cb.c | 22 +++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c index bd39586abd5a..f3c61ebad323 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c @@ -4,6 +4,7 @@ #include #include "struct_ops_module.skel.h" +#include "struct_ops_nulled_out_cb.skel.h" static void check_map_info(struct bpf_map_info *info) { @@ -174,6 +175,30 @@ static void test_struct_ops_incompatible(void) struct_ops_module__destroy(skel); } +/* validate that it's ok to "turn off" callback that kernel supports */ +static void test_struct_ops_nulled_out_cb(void) +{ + struct struct_ops_nulled_out_cb *skel; + int err; + + skel = struct_ops_nulled_out_cb__open(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + return; + + /* kernel knows about test_1, but we still null it out */ + skel->struct_ops.ops->test_1 = NULL; + + err = struct_ops_nulled_out_cb__load(skel); + if (!ASSERT_OK(err, "skel_load")) + goto cleanup; + + ASSERT_FALSE(bpf_program__autoload(skel->progs.test_1_turn_off), "prog_autoload"); + ASSERT_LT(bpf_program__fd(skel->progs.test_1_turn_off), 0, "prog_fd"); + +cleanup: + struct_ops_nulled_out_cb__destroy(skel); +} + void serial_test_struct_ops_module(void) { if (test__start_subtest("test_struct_ops_load")) @@ -182,5 +207,7 @@ void serial_test_struct_ops_module(void) test_struct_ops_not_zeroed(); if (test__start_subtest("test_struct_ops_incompatible")) test_struct_ops_incompatible(); + if (test__start_subtest("test_struct_ops_null_out_cb")) + test_struct_ops_nulled_out_cb(); } diff --git a/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c b/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c new file mode 100644 index 000000000000..fa2021388485 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/struct_ops_nulled_out_cb.c @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ +#include +#include +#include "../bpf_testmod/bpf_testmod.h" + +char _license[] SEC("license") = "GPL"; + +int rand; +int arr[1]; + +SEC("struct_ops/test_1") +int BPF_PROG(test_1_turn_off) +{ + return arr[rand]; /* potentially way out of range access */ +} + +SEC(".struct_ops.link") +struct bpf_testmod_ops ops = { + .test_1 = (void *)test_1_turn_off, +}; + From patchwork Tue May 7 00:13:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656091 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B899810FA for ; Tue, 7 May 2024 00:13:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040832; cv=none; b=QnZyEPeTinx+kcJGQId1YPmMM2ErsfGZ1KpIxfjNFzN4nlekaK9tJxcnvv5cedsocnsMoRO3qBvIOj1zNfazfDtT4zDVtNOMaGXonGztucF+bLbXEWnI2UWZG3WRMBvQ9vDK5C5t8BeMqJefwrDiiOzL3uIKg7AgEMRhVC1vICQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040832; c=relaxed/simple; bh=/xZHCP4fsyVjIySO6TVQTq8s5ppUOvO0ovzCQoojzSI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bo8CiooRwYHqHzM5Wc8JxCF92t36DH/A+2xcpzKGvLAddIcq0ihuvk25P7gPoLuAJ74aFQfbCm4r/ZY1DuQAiaOG3omWrhRtogydQZrh6KHAyTUmIoi4WhDi/BndvANYTiVX1WJPHauKgI5Ob2x+zXZWGxFVsubdvMITjt03r80= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=J4l93vsK; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="J4l93vsK" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 21C56C116B1; Tue, 7 May 2024 00:13:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040832; bh=/xZHCP4fsyVjIySO6TVQTq8s5ppUOvO0ovzCQoojzSI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=J4l93vsKUu4GeWTplCyRDK4jISPw740rqAu3cd0bpI6UCT2mv3AsU5Fm9iPPRji6h sXtpkktfqfNwgrrCgNzLiqFZcFLrKY2kFc1j+UYm2F938Yb/AUJIxFZHvkWr7uWM3Q Ggf4899w5Vgin9QnHY2RqJVSm/rZxu5NGxXWGIzLktV1RaGCG8rjNujRWFvwHHtSaG mzfZy/BgqySIRoEc9fqvkKPenHFWl36TOsWLfFqiRGtpt09hqx7fOhp3DTF0iPncgt q6r4ZxyIKHfANOxDOA0X7MEsPGa7YNskQOo/dtsO8kyyhd3mtd3jCo2eymV3Axa832 GfFQFRuzvlZGQ== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 4/7] libbpf: fix libbpf_strerror_r() handling unknown errors Date: Mon, 6 May 2024 17:13:32 -0700 Message-ID: <20240507001335.1445325-5-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net strerror_r(), used from libbpf-specific libbpf_strerror_r() wrapper is documented to return error in two different ways, depending on glibc version. Take that into account when handling strerror_r()'s own errors, which happens when we pass some non-standard (internal) kernel error to it. Before this patch we'd have "ERROR: strerror_r(524)=22", which is quite confusing. Now for the same situation we'll see a bit less visually scary "unknown error (-524)". At least we won't confuse user with irrelevant EINVAL (22). Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/str_error.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c index 146da01979c7..5e6a1e27ddf9 100644 --- a/tools/lib/bpf/str_error.c +++ b/tools/lib/bpf/str_error.c @@ -2,6 +2,7 @@ #undef _GNU_SOURCE #include #include +#include #include "str_error.h" /* make sure libbpf doesn't use kernel-only integer typedefs */ @@ -15,7 +16,18 @@ char *libbpf_strerror_r(int err, char *dst, int len) { int ret = strerror_r(err < 0 ? -err : err, dst, len); - if (ret) - snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret); + /* on glibc <2.13, ret == -1 and errno is set, if strerror_r() can't + * handle the error, on glibc >=2.13 *positive* (errno-like) error + * code is returned directly + */ + if (ret == -1) + ret = errno; + if (ret) { + if (ret == EINVAL) + /* strerror_r() doesn't recognize this specific error */ + snprintf(dst, len, "unknown error (%d)", err < 0 ? err : -err); + else + snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret); + } return dst; } From patchwork Tue May 7 00:13:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656092 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 00731653 for ; Tue, 7 May 2024 00:13:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040836; cv=none; b=otKVEc0OEVzcPVCG0tjHZboDzARJxWtcSlQzFvUt5IcIYMOuJajvJ3E0HEbqnd1UP/p0AF7wXnTRn6678w814Co5cu4J7M4HSql2kJ1B6rD1XmbECDQ8dGxyyDYVEXj93rCnoub16rhMUh8Q8/KIPP8I5SaJR11ikel9FxgKN9k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040836; c=relaxed/simple; bh=Qfo7/DmAqnVQb+lryVOFU9WxqfPwY+i4xDZT3A3xM6I=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=FmpBXcyDvoD0I8fJTn4X0IcQTpCswyDlip6shNPGhBA2NU1zCuQ1NhIMUmXZvhXpOIv1gHQ1IMz53ERcBQMV6m4AUbPMWEG5Z8Hpl7yED28xv9rtlMOU+iI1z9YiFqOlU4wO5LYu0YG8ec19Wn1WPEBR4sDsf43dzQpN6tXZrAo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Xi/D79YA; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Xi/D79YA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 60C50C116B1; Tue, 7 May 2024 00:13:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040835; bh=Qfo7/DmAqnVQb+lryVOFU9WxqfPwY+i4xDZT3A3xM6I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Xi/D79YAthcC8LIfkI5LlAh14Zz3ppm85Ia8+cBxeIjhBbCDhN5wNbGrH1C/ywz9E 5ZbrP7Q8Arlx1CRVBZKUOQgEKhJrAH3eEiu6tz6vJP0ita1p6uf6X9wlqD8ODEB3u1 k9lwelQ7Or9BuPFB3/TmbtdwZM7rLAKRWVe1DF/Od7yj03Q3F2NQ3UOcnxvDcRIuja ROj1/7f6mv4GYYXgCZ07pU4JEgx5Lamv9kvA5ba63xV640H3U93p00QRsHLDIX9Maf NT/zb/pSTa7AVN1gzW+kEaGj9wCqOM5OVAChU88Sr3b5TwVS3mB2fuViYcMqj0MjK6 bOxLHjk5YQNHg== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com, Tejun Heo Subject: [PATCH bpf-next 5/7] libbpf: improve early detection of doomed-to-fail BPF program loading Date: Mon, 6 May 2024 17:13:33 -0700 Message-ID: <20240507001335.1445325-6-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Extend libbpf's pre-load checks for BPF programs, detecting more typical conditions that are destinated to cause BPF program failure. This is an opportunity to provide more helpful and actionable error message to users, instead of potentially very confusing BPF verifier log and/or error. In this case, we detect struct_ops BPF program that was not referenced anywhere, but still attempted to be loaded (according to libbpf logic). Suggest that the program might need to be used in some struct_ops variable. User will get a message of the following kind: libbpf: prog 'test_1_forgotten': SEC("struct_ops") program isn't referenced anywhere, did you forget to use it? Suggested-by: Tejun Heo Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/libbpf.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 04de4fb81785..5401f2df463d 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -7372,7 +7372,11 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog __u32 log_level = prog->log_level; int ret, err; - if (prog->type == BPF_PROG_TYPE_UNSPEC) { + /* Be more helpful by rejecting programs that can't be validated early + * with more meaningful and actionable error message. + */ + switch (prog->type) { + case BPF_PROG_TYPE_UNSPEC: /* * The program type must be set. Most likely we couldn't find a proper * section definition at load time, and thus we didn't infer the type. @@ -7380,6 +7384,15 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog pr_warn("prog '%s': missing BPF prog type, check ELF section name '%s'\n", prog->name, prog->sec_name); return -EINVAL; + case BPF_PROG_TYPE_STRUCT_OPS: + if (prog->attach_btf_id == 0) { + pr_warn("prog '%s': SEC(\"struct_ops\") program isn't referenced anywhere, did you forget to use it?\n", + prog->name); + return -EINVAL; + } + break; + default: + break; } if (!insns || !insns_cnt) From patchwork Tue May 7 00:13:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656093 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 32992389 for ; Tue, 7 May 2024 00:13:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040839; cv=none; b=d6Y5Qad7wH2RK5s0gWgKN1AXtzG/7CL4XGpTzLHG/16ZpMRY3aIW8eilfjPj3l6t88Ke9uALnlzgyWbwB9hGHrHubYstZjDfJgGdc1yOV/wOtXpbInONlmXgovEksh729idQg6idxRSrtmAmS+JsPTRQn/5R3XyKsBsJ2b6GGEw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040839; c=relaxed/simple; bh=NadnHBQ3TBYENzaBm12S5ll5QN2eMBpcavPtAqhYpw0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=l5/+UdfInPYI/1PkM3dwPblPiVFmS0RiCCuYZpjJMHzyhnWg2gy5NRIZBsMWUrP1Rm4FUz4bwWEK2ZjT4qYPEo2L+Mn/UKhQKFOSF8x5y2cfBxVEFmUxssUJddw5fIUgKzpRRt4iIVmjLpJyNRzqqwPM2c8aBDXxJ+xzz+rg/bQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=W75IATbt; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="W75IATbt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9817EC116B1; Tue, 7 May 2024 00:13:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040838; bh=NadnHBQ3TBYENzaBm12S5ll5QN2eMBpcavPtAqhYpw0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W75IATbtvaG84uUeGmehk/H05lw7AmXucrmfhkGwhSnih9Vl6UFAKfrFNkkz6pSjJ 4QPmQXONZiyx2+7/N7Q5Eq68Vx6lVnoOuliViLi4AAJwcat1RFGTFHeo8MOKDO6qRu tVcmgnoDONn/bDsgC81LS9i6wArEGmQKenm5KVBzhwSFhFZhvWDzFFtOuH5fwM7mog yAdFfv/vDYo4vxhM+l4IZAGxwDd1pHuE2PDV0HDAwAFmXelI2pUaz3Cbkavf7azorp jxT9B4MgAnbig2UPP+WZm/YMT5dnis/3EJOqpFO05YOygWt4tYBWEDpZJrrumPDicm MDx2FTfC5KBnw== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 6/7] selftests/bpf: validate struct_ops early failure detection logic Date: Mon, 6 May 2024 17:13:34 -0700 Message-ID: <20240507001335.1445325-7-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Add a simple test that validates that libbpf will reject isolated struct_ops program early with helpful warning message. Also validate that explicit use of such BPF program through BPF skeleton after BPF object is open won't trigger any warnings. Signed-off-by: Andrii Nakryiko --- .../bpf/prog_tests/test_struct_ops_module.c | 45 +++++++++++++++++++ .../bpf/progs/struct_ops_forgotten_cb.c | 19 ++++++++ 2 files changed, 64 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c index f3c61ebad323..3785b648c8ad 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c @@ -5,6 +5,7 @@ #include "struct_ops_module.skel.h" #include "struct_ops_nulled_out_cb.skel.h" +#include "struct_ops_forgotten_cb.skel.h" static void check_map_info(struct bpf_map_info *info) { @@ -199,6 +200,48 @@ static void test_struct_ops_nulled_out_cb(void) struct_ops_nulled_out_cb__destroy(skel); } +/* validate that libbpf generates reasonable error message if struct_ops is + * not referenced in any struct_ops map + */ +static void test_struct_ops_forgotten_cb(void) +{ + struct struct_ops_forgotten_cb *skel; + char *log; + int err; + + skel = struct_ops_forgotten_cb__open(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + return; + + start_libbpf_log_capture(); + + err = struct_ops_forgotten_cb__load(skel); + if (!ASSERT_ERR(err, "skel_load")) + goto cleanup; + + log = stop_libbpf_log_capture(); + ASSERT_HAS_SUBSTR(log, + "prog 'test_1_forgotten': SEC(\"struct_ops\") program isn't referenced anywhere, did you forget to use it?", + "libbpf_log"); + free(log); + + struct_ops_forgotten_cb__destroy(skel); + + /* now let's programmatically use it, we should be fine now */ + skel = struct_ops_forgotten_cb__open(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + return; + + skel->struct_ops.ops->test_1 = skel->progs.test_1_forgotten; /* not anymore */ + + err = struct_ops_forgotten_cb__load(skel); + if (!ASSERT_OK(err, "skel_load")) + goto cleanup; + +cleanup: + struct_ops_forgotten_cb__destroy(skel); +} + void serial_test_struct_ops_module(void) { if (test__start_subtest("test_struct_ops_load")) @@ -209,5 +252,7 @@ void serial_test_struct_ops_module(void) test_struct_ops_incompatible(); if (test__start_subtest("test_struct_ops_null_out_cb")) test_struct_ops_nulled_out_cb(); + if (test__start_subtest("struct_ops_forgotten_cb")) + test_struct_ops_forgotten_cb(); } diff --git a/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c b/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c new file mode 100644 index 000000000000..3c822103bd40 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/struct_ops_forgotten_cb.c @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ +#include +#include +#include "../bpf_testmod/bpf_testmod.h" + +char _license[] SEC("license") = "GPL"; + +SEC("struct_ops/test_1") +int BPF_PROG(test_1_forgotten) +{ + return 0; +} + +SEC(".struct_ops.link") +struct bpf_testmod_ops ops = { + /* we forgot to reference test_1_forgotten above, oops */ +}; + From patchwork Tue May 7 00:13:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13656094 X-Patchwork-Delegate: bpf@iogearbox.net Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 79715182 for ; Tue, 7 May 2024 00:14:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040842; cv=none; b=sC3nEAGB7WO6U/Gm0Z1VUTKkp52sO1eCLcZM3vOHgvrV1SzqAGbf1PMj4Blxwq2oh5JdkhZbWumikDhMpxpLmqJ8UIZQrgPl+tCkktCk7MYN8so9KXLn6j646UHFhoIDkU/LOB9xFCqFL4yxm8t9MuTiMrvV5OjpPQyyKbIH6+k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715040842; c=relaxed/simple; bh=dQQq9QxTiVWzy4mMz4Rznb6ePGi0v3bayiJutnO3pcg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=XPOfVQe6M7oP7/YecVty2jieDTJEAWo2e/EsLDjIvKBqwe8g5bq9heJ7tS2WfkIKQxu1Wicv/lux1UyTXiR5evCeCTrtGi9jFFIQlXDmmfK1W7V5Wu45snCil0wkr/6lkkDFVOMD3sH8LPegMRByCvRIhh2QccjENRoz9MD0nbk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hEWJd9QQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hEWJd9QQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D66E2C3277B; Tue, 7 May 2024 00:14:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715040842; bh=dQQq9QxTiVWzy4mMz4Rznb6ePGi0v3bayiJutnO3pcg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hEWJd9QQRmwrIOE+QSC2KrShbB2HLYptfubGyMB0mk8UlpK73HZUQlYuOeyj1+fuc c+GYXH8CUGgrJX8DhEGH7oA9qQ5QFQdSJmrVz3pDs66sQAqbEYSJisOvsIdTgl9dj4 hDgQ6n/T07umifVP/GsfzdlP6Yk+PgEHn0P4NrkkRcW6fFeqWPGww7ocXSAkNGU4LB 0RxrpYDZc4yFGYMVI8MuzrUhIAu+bMIbVR2prNWB/l7CGxNbjzDShZ8O5zNXGOdR+s XKkr22UNiTULTv5/yaV44Qp5sCziZUjeTm1PooF3wM/g+nqY5UC7vqJWJEMAephcrg csPdnNzpqtJiA== From: Andrii Nakryiko To: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, martin.lau@kernel.org Cc: andrii@kernel.org, kernel-team@meta.com Subject: [PATCH bpf-next 7/7] selftests/bpf: shorten subtest names for struct_ops_module test Date: Mon, 6 May 2024 17:13:35 -0700 Message-ID: <20240507001335.1445325-8-andrii@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240507001335.1445325-1-andrii@kernel.org> References: <20240507001335.1445325-1-andrii@kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Drive-by clean up, we shouldn't use meaningless "test_" prefix for subtest names. Signed-off-by: Andrii Nakryiko --- .../selftests/bpf/prog_tests/test_struct_ops_module.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c index 3785b648c8ad..29e183a80f49 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c @@ -244,13 +244,13 @@ static void test_struct_ops_forgotten_cb(void) void serial_test_struct_ops_module(void) { - if (test__start_subtest("test_struct_ops_load")) + if (test__start_subtest("struct_ops_load")) test_struct_ops_load(); - if (test__start_subtest("test_struct_ops_not_zeroed")) + if (test__start_subtest("struct_ops_not_zeroed")) test_struct_ops_not_zeroed(); - if (test__start_subtest("test_struct_ops_incompatible")) + if (test__start_subtest("struct_ops_incompatible")) test_struct_ops_incompatible(); - if (test__start_subtest("test_struct_ops_null_out_cb")) + if (test__start_subtest("struct_ops_null_out_cb")) test_struct_ops_nulled_out_cb(); if (test__start_subtest("struct_ops_forgotten_cb")) test_struct_ops_forgotten_cb();