From patchwork Thu Jan 18 03:31:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13522350 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 980386104 for ; Thu, 18 Jan 2024 03:31: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=1705548709; cv=none; b=AL+LgTMtRgwDTYfeQTcLM/0cVVXFQvEQeJb6EEePVQPmhhSCyOlkmFW/ByU3uRpF2yedYz0nnJHP1EyLEi4fTBatGi5hrpxc4c9+Zdsj2al/U/oq8bICMJZ/Ub1qo3HBbUy4mthZj1bu3Pp7oZ+6O47tyDlQO3ncQUV0f1m2Oj4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705548709; c=relaxed/simple; bh=BMz6Xlfjn8W0mPyVx/S5zH7KppTk0vFZ+ineoLzH2Ls=; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-Id: X-Mailer:In-Reply-To:References:MIME-Version: Content-Transfer-Encoding; b=iQ8taUpL/wiCat5xcSBFMEyqucxKPwIuATDBLjZKoE4ki1V49NdDxC8hbNHGLdiXL/ZI1B2RoJXCgkHpXpyDhT+1GWHaOcTjYwTH4W+uos7V19taxTFo8fJ+l9+Y6+7Bp7K8zkfAbCazzohvBR3F4KX3w9ccGu2wEZLHF7YDwX0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=EWpgj0Zn; 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="EWpgj0Zn" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E580FC433C7; Thu, 18 Jan 2024 03:31:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705548709; bh=BMz6Xlfjn8W0mPyVx/S5zH7KppTk0vFZ+ineoLzH2Ls=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EWpgj0Zn9fUEAD4sBPchowEV8HFcWiTyWwUvNFnhpB1omWQ0rH76XbsT/ErktDv2x mO2yO7iWeUMpa+fFW8OjiGhO4xPz98j7FqrwES/vypYnGXCI4sKVgBBFIHPigIIk5Y ER8d0bU/pogfEsn52MTjioKfFQwJ+YSyac7XplIuLEPCQyHrAx+07FTerbn4hcMTHR Bt4BrEzWC3e2Bm00swZvZUT6ufe8ViGaGuWW7CqFSRV6dbZwfkIri2v+DkPcYWoxn3 X0Dv4IJjIQ+Ro0td5qneDylne1OR75Vl+LV+FuRtVARiSIZrNjn4TQKEfXXx5crllr F8pCZt4Iv/iNQ== 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 v3 bpf 1/5] libbpf: feature-detect arg:ctx tag support in kernel Date: Wed, 17 Jan 2024 19:31:39 -0800 Message-Id: <20240118033143.3384355-2-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org> References: <20240118033143.3384355-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 feature detector of kernel-side arg:ctx (__arg_ctx) tag support. If this is detected, libbpf will avoid doing any __arg_ctx-related BTF rewriting and checks in favor of letting kernel handle this completely. test_global_funcs/ctx_arg_rewrite subtest is adjusted to do the same feature detection (albeit in much simpler, though round-about and inefficient, way), and skip the tests. This is done to still be able to execute this test on older kernels (like in libbpf CI). Note, BPF token series ([0]) does a major refactor and code moving of libbpf-internal feature detection "framework", so to avoid unnecessary conflicts we keep newly added feature detection stand-alone with ad-hoc result caching. Once things settle, there will be a small follow up to re-integrate everything back and move code into its final place in newly-added (by BPF token series) features.c file. [0] https://patchwork.kernel.org/project/netdevbpf/list/?series=814209&state=* Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/libbpf.c | 67 +++++++++++++++++++ .../bpf/prog_tests/test_global_funcs.c | 13 ++++ 2 files changed, 80 insertions(+) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index c5a42ac309fd..61db92189517 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -6757,6 +6757,69 @@ static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_progr return fn_id; } +static int probe_kern_arg_ctx_tag(void) +{ + /* To minimize merge conflicts with BPF token series that refactors + * feature detection code a lot, we don't integrate + * probe_kern_arg_ctx_tag() into kernel_supports() feature-detection + * framework yet, doing our own caching internally. + * This will be cleaned up a bit later when bpf/bpf-next trees settle. + */ + static int cached_result = -1; + static const char strs[] = "\0a\0b\0arg:ctx\0"; + const __u32 types[] = { + /* [1] INT */ + BTF_TYPE_INT_ENC(1 /* "a" */, BTF_INT_SIGNED, 0, 32, 4), + /* [2] PTR -> VOID */ + BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0), + /* [3] FUNC_PROTO `int(void *a)` */ + BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1), + BTF_PARAM_ENC(1 /* "a" */, 2), + /* [4] FUNC 'a' -> FUNC_PROTO (main prog) */ + BTF_TYPE_ENC(1 /* "a" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 3), + /* [5] FUNC_PROTO `int(void *b __arg_ctx)` */ + BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1), + BTF_PARAM_ENC(3 /* "b" */, 2), + /* [6] FUNC 'b' -> FUNC_PROTO (subprog) */ + BTF_TYPE_ENC(3 /* "b" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 5), + /* [7] DECL_TAG 'arg:ctx' -> func 'b' arg 'b' */ + BTF_TYPE_DECL_TAG_ENC(5 /* "arg:ctx" */, 6, 0), + }; + const struct bpf_insn insns[] = { + /* main prog */ + BPF_CALL_REL(+1), + BPF_EXIT_INSN(), + /* global subprog */ + BPF_EMIT_CALL(BPF_FUNC_get_func_ip), /* needs PTR_TO_CTX */ + BPF_EXIT_INSN(), + }; + const struct bpf_func_info_min func_infos[] = { + { 0, 4 }, /* main prog -> FUNC 'a' */ + { 2, 6 }, /* subprog -> FUNC 'b' */ + }; + LIBBPF_OPTS(bpf_prog_load_opts, opts); + int prog_fd, btf_fd, insn_cnt = ARRAY_SIZE(insns); + + if (cached_result >= 0) + return cached_result; + + btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs)); + if (btf_fd < 0) + return 0; + + opts.prog_btf_fd = btf_fd; + opts.func_info = &func_infos; + opts.func_info_cnt = ARRAY_SIZE(func_infos); + opts.func_info_rec_size = sizeof(func_infos[0]); + + prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, "det_arg_ctx", + "GPL", insns, insn_cnt, &opts); + close(btf_fd); + + cached_result = probe_fd(prog_fd); + return cached_result; +} + /* Check if main program or global subprog's function prototype has `arg:ctx` * argument tags, and, if necessary, substitute correct type to match what BPF * verifier would expect, taking into account specific program type. This @@ -6780,6 +6843,10 @@ static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_progra if (!obj->btf_ext || !prog->func_info) return 0; + /* don't do any fix ups if kernel natively supports __arg_ctx */ + if (probe_kern_arg_ctx_tag() > 0) + return 0; + /* some BPF program types just don't have named context structs, so * this fallback mechanism doesn't work for them */ diff --git a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c index 67d4ef9e62b3..e905cbaf6b3d 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c +++ b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c @@ -47,6 +47,19 @@ static void subtest_ctx_arg_rewrite(void) struct btf *btf = NULL; __u32 info_len = sizeof(info); int err, fd, i; + struct btf *kern_btf = NULL; + + kern_btf = btf__load_vmlinux_btf(); + if (!ASSERT_OK_PTR(kern_btf, "kern_btf_load")) + return; + + /* simple detection of kernel native arg:ctx tag support */ + if (btf__find_by_name_kind(kern_btf, "bpf_subprog_arg_info", BTF_KIND_STRUCT) > 0) { + test__skip(); + btf__free(kern_btf); + return; + } + btf__free(kern_btf); skel = test_global_func_ctx_args__open(); if (!ASSERT_OK_PTR(skel, "skel_open")) From patchwork Thu Jan 18 03:31:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13522351 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 D04E723D9 for ; Thu, 18 Jan 2024 03:31: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=1705548712; cv=none; b=iflzpRDiFH1nR2B8X72KPsGQC1ay28NYqMvuULtyxHqXFgSU+PabOscG9iGgNgsFjyEm03juuMFvAPKlr2hahibYMLJhImMaQYQa3L3Vao66RQao4oAHsOy21fJqfBXwrcc5ZFso7zB8PqrtRsFmjc4RQX15i/nDUxHE7rBANdQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705548712; c=relaxed/simple; bh=3nywmZMRjUBXXGZwrog1A4iNlZe5AfGFE5M7bTzPOj8=; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-Id: X-Mailer:In-Reply-To:References:MIME-Version: Content-Transfer-Encoding; b=U2rPyMinM6MMpWKFQLcDDHC+XwTAnZdQlYDW8r3Ig1mrAk3Sa2w/nkHueEw4rU9735jEN+tF+qXPqfPJqIqlW12uhA4VX88Jd9agsXMDF/xVGQdf+llFve6nIyFiu+s4sGstiyNEQQzAFDuOj9Jo6tpANcaYE0j2xEKWNe17qZE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=FDAHEK+m; 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="FDAHEK+m" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3BFF3C433F1; Thu, 18 Jan 2024 03:31:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705548712; bh=3nywmZMRjUBXXGZwrog1A4iNlZe5AfGFE5M7bTzPOj8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FDAHEK+mHHY0MSZpV1xls7uJZvv/OviH8wHY9ekwhIH2czOGuVOVeYt9/NdyARhkf cJi/RXTct1lf+zSdDo/h+rve623VShsEL20RfbdkvBhmcFSnvpQ6rJC1ZSC/QxxwH8 njJfKeFvEisSac+82mAVwf/7H87T04PiwRVCQxxf35uXALNmB37hUZD2l7GrKs8ko9 MH8hb+AYpypfgfG5xZ98i/0he3mD7OY/UWhLzAoAuXTDV8Oy3h4Ddunb+GK7nMkSH1 44N4qUMHoCxfGbL6+p9DuCoO/x9vLiezyrA/u79lsBpSRLJK1NwriHxQtuJevbN6Jp s7V1tcC/fTM+g== 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 v3 bpf 2/5] bpf: extract bpf_ctx_convert_map logic and make it more reusable Date: Wed, 17 Jan 2024 19:31:40 -0800 Message-Id: <20240118033143.3384355-3-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org> References: <20240118033143.3384355-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 Refactor btf_get_prog_ctx_type() a bit to allow reuse of bpf_ctx_convert_map logic in more than one places. Simplify interface by returning btf_type instead of btf_member (field reference in BTF). To do the above we need to touch and start untangling btf_translate_to_vmlinux() implementation. We do the bare minimum to not regress anything for btf_translate_to_vmlinux(), but its implementation is very questionable for what it claims to be doing. Mapping kfunc argument types to kernel corresponding types conceptually is quite different from recognizing program context types. Fixing this is out of scope for this change though. Signed-off-by: Andrii Nakryiko --- include/linux/btf.h | 2 +- kernel/bpf/btf.c | 71 ++++++++++++++++++++++++++++----------------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/include/linux/btf.h b/include/linux/btf.h index 59d404e22814..cf5c6ff48981 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -512,7 +512,7 @@ s32 btf_find_dtor_kfunc(struct btf *btf, u32 btf_id); int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_cnt, struct module *owner); struct btf_struct_meta *btf_find_struct_meta(const struct btf *btf, u32 btf_id); -const struct btf_member * +const struct btf_type * btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, const struct btf_type *t, enum bpf_prog_type prog_type, int arg); diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 51e8b4bee0c8..10ac9efc662d 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5615,21 +5615,46 @@ static u8 bpf_ctx_convert_map[] = { #undef BPF_MAP_TYPE #undef BPF_LINK_TYPE -const struct btf_member * -btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, - const struct btf_type *t, enum bpf_prog_type prog_type, - int arg) +static const struct btf_type *find_canonical_prog_ctx_type(enum bpf_prog_type prog_type) { const struct btf_type *conv_struct; - const struct btf_type *ctx_struct; const struct btf_member *ctx_type; - const char *tname, *ctx_tname; conv_struct = bpf_ctx_convert.t; - if (!conv_struct) { - bpf_log(log, "btf_vmlinux is malformed\n"); + if (!conv_struct) return NULL; - } + /* prog_type is valid bpf program type. No need for bounds check. */ + ctx_type = btf_type_member(conv_struct) + bpf_ctx_convert_map[prog_type] * 2; + /* ctx_type is a pointer to prog_ctx_type in vmlinux. + * Like 'struct __sk_buff' + */ + return btf_type_by_id(btf_vmlinux, ctx_type->type); +} + +static int find_kern_ctx_type_id(enum bpf_prog_type prog_type) +{ + const struct btf_type *conv_struct; + const struct btf_member *ctx_type; + + conv_struct = bpf_ctx_convert.t; + if (!conv_struct) + return -EFAULT; + /* prog_type is valid bpf program type. No need for bounds check. */ + ctx_type = btf_type_member(conv_struct) + bpf_ctx_convert_map[prog_type] * 2 + 1; + /* ctx_type is a pointer to prog_ctx_type in vmlinux. + * Like 'struct sk_buff' + */ + return ctx_type->type; +} + +const struct btf_type * +btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, + const struct btf_type *t, enum bpf_prog_type prog_type, + int arg) +{ + const struct btf_type *ctx_type; + const char *tname, *ctx_tname; + t = btf_type_by_id(btf, t->type); while (btf_type_is_modifier(t)) t = btf_type_by_id(btf, t->type); @@ -5646,17 +5671,15 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, bpf_log(log, "arg#%d struct doesn't have a name\n", arg); return NULL; } - /* prog_type is valid bpf program type. No need for bounds check. */ - ctx_type = btf_type_member(conv_struct) + bpf_ctx_convert_map[prog_type] * 2; - /* ctx_struct is a pointer to prog_ctx_type in vmlinux. - * Like 'struct __sk_buff' - */ - ctx_struct = btf_type_by_id(btf_vmlinux, ctx_type->type); - if (!ctx_struct) + + ctx_type = find_canonical_prog_ctx_type(prog_type); + if (!ctx_type) { + bpf_log(log, "btf_vmlinux is malformed\n"); /* should not happen */ return NULL; + } again: - ctx_tname = btf_name_by_offset(btf_vmlinux, ctx_struct->name_off); + ctx_tname = btf_name_by_offset(btf_vmlinux, ctx_type->name_off); if (!ctx_tname) { /* should not happen */ bpf_log(log, "Please fix kernel include/linux/bpf_types.h\n"); @@ -5677,10 +5700,10 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, /* bpf_user_pt_regs_t is a typedef, so resolve it to * underlying struct and check name again */ - if (!btf_type_is_modifier(ctx_struct)) + if (!btf_type_is_modifier(ctx_type)) return NULL; - while (btf_type_is_modifier(ctx_struct)) - ctx_struct = btf_type_by_id(btf_vmlinux, ctx_struct->type); + while (btf_type_is_modifier(ctx_type)) + ctx_type = btf_type_by_id(btf_vmlinux, ctx_type->type); goto again; } return ctx_type; @@ -5692,13 +5715,9 @@ static int btf_translate_to_vmlinux(struct bpf_verifier_log *log, enum bpf_prog_type prog_type, int arg) { - const struct btf_member *prog_ctx_type, *kern_ctx_type; - - prog_ctx_type = btf_get_prog_ctx_type(log, btf, t, prog_type, arg); - if (!prog_ctx_type) + if (!btf_get_prog_ctx_type(log, btf, t, prog_type, arg)) return -ENOENT; - kern_ctx_type = prog_ctx_type + 1; - return kern_ctx_type->type; + return find_kern_ctx_type_id(prog_type); } int get_kern_ctx_btf_id(struct bpf_verifier_log *log, enum bpf_prog_type prog_type) From patchwork Thu Jan 18 03:31:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13522352 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 1B7486FBC for ; Thu, 18 Jan 2024 03:31: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=1705548716; cv=none; b=oyPM6uwMGDHkRP2XWt1HQW7+W9xI44qIVgl9oRNbFvKt6FXkJrG6Woi2w8yQ7ORj0aQQY7Pn4VjZV8etq4vx6AY5oKjMy4uEQ7EUfBG+1WuqMBvaAsbduIaqAPwqGTO7jXumwt30U95cDf92WqFmlh2F++fqvprIScwqYnrCEck= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705548716; c=relaxed/simple; bh=Xzmi2CjSg8UA6eWmEFRIr/rMVqLUpIugr1M2WiWbxyI=; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-Id: X-Mailer:In-Reply-To:References:MIME-Version: Content-Transfer-Encoding; b=JQCq+Up/uigpic+DhqWFE/6D8JUXar+RB5zW84pIY3oQfkDdTPhSazQs7oMVwhnsrC0RD7b83Yy1D4/pnTYtWewgtPq7wLQ651L7Ks2U/xjjitolPMorLAIPTgMybM2/hM1iEzRK09NHr5OhEw6ZQCG/iuRKVRr+0i+J9raPvT8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HSSeSLYq; 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="HSSeSLYq" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 68A9AC43390; Thu, 18 Jan 2024 03:31:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705548715; bh=Xzmi2CjSg8UA6eWmEFRIr/rMVqLUpIugr1M2WiWbxyI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HSSeSLYqiJMLMswe3uR1y8ssYOPVxqX8uy4p0YCDFpelqoXQD0nDZkd2jGVIZywHU Xq+WdYQdPG3GnEIMaiL+tqV/8/4QjCF4H4PD7ZXwe/DQnnvFafM4RDDu66Zp8zI5dD 3/WktlQUPQgtqVIYT48o5T4U58bNkbO1bas5U0K2RNQTNHdq840zgbgxOeEBHGUHEJ ySaJEg/7VgEMG5IsydKqCVlxtwoSQCPJ80cIfwGCkSsC5guKADZIqSXda2SM1nzWIg 98iRgEJAxTnDM6JXWmO5pP8p5Rn8gwkOB6LHCrs2f5aINZ+LScl9SBO3+Yvn6PGuJ2 L9XYFboCQaH7Q== 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 v3 bpf 3/5] bpf: enforce types for __arg_ctx-tagged arguments in global subprogs Date: Wed, 17 Jan 2024 19:31:41 -0800 Message-Id: <20240118033143.3384355-4-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org> References: <20240118033143.3384355-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 enforcement of expected types for context arguments tagged with arg:ctx (__arg_ctx) tag. First, any program type will accept generic `void *` context type when combined with __arg_ctx tag. Besides accepting "canonical" struct names and `void *`, for a bunch of program types for which program context is actually a named struct, we allows a bunch of pragmatic exceptions to match real-world and expected usage: - for both kprobes and perf_event we allow `bpf_user_pt_regs_t *` as canonical context argument type, where `bpf_user_pt_regs_t` is a *typedef*, not a struct; - for kprobes, we also always accept `struct pt_regs *`, as that's what actually is passed as a context to any kprobe program; - for perf_event, we resolve typedefs (unless it's `bpf_user_pt_regs_t`) down to actual struct type and accept `struct pt_regs *`, or `struct user_pt_regs *`, or `struct user_regs_struct *`, depending on the actual struct type kernel architecture points `bpf_user_pt_regs_t` typedef to; otherwise, canonical `struct bpf_perf_event_data *` is expected; - for raw_tp/raw_tp.w programs, `u64/long *` are accepted, as that's what's expected with BPF_PROG() usage; otherwise, canonical `struct bpf_raw_tracepoint_args *` is expected; - tp_btf supports both `struct bpf_raw_tracepoint_args *` and `u64 *` formats, both are coded as expections as tp_btf is actually a TRACING program type, which has no canonical context type; - iterator programs accept `struct bpf_iter__xxx *` structs, currently with no further iterator-type specific enforcement; - fentry/fexit/fmod_ret/lsm/struct_ops all accept `u64 *`; - classic tracepoint programs, as well as syscall and freplace programs allow any user-provided type. In all other cases kernel will enforce exact match of struct name to expected canonical type. And if user-provided type doesn't match that expectation, verifier will emit helpful message with expected type name. Note a bit unnatural way the check is done after processing all the arguments. This is done to avoid conflict between bpf and bpf-next trees. Once trees converge, a small follow up patch will place a simple btf_validate_prog_ctx_type() check into a proper ARG_PTR_TO_CTX branch (which bpf-next tree patch refactored already), removing duplicated arg:ctx detection logic. Suggested-by: Alexei Starovoitov Signed-off-by: Andrii Nakryiko --- kernel/bpf/btf.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 10ac9efc662d..596471189176 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5709,6 +5709,149 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, return ctx_type; } +/* forward declarations for arch-specific underlying types of + * bpf_user_pt_regs_t; this avoids the need for arch-specific #ifdef + * compilation guards below for BPF_PROG_TYPE_PERF_EVENT checks, but still + * works correctly with __builtin_types_compatible_p() on respective + * architectures + */ +struct user_regs_struct; +struct user_pt_regs; + +static int btf_validate_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf, + const struct btf_type *t, int arg, + enum bpf_prog_type prog_type, + enum bpf_attach_type attach_type) +{ + const struct btf_type *ctx_type; + const char *tname, *ctx_tname; + + if (!btf_is_ptr(t)) { + bpf_log(log, "arg#%d type isn't a pointer\n", arg); + return -EINVAL; + } + t = btf_type_by_id(btf, t->type); + + /* KPROBE and PERF_EVENT programs allow bpf_user_pt_regs_t typedef */ + if (prog_type == BPF_PROG_TYPE_KPROBE || prog_type == BPF_PROG_TYPE_PERF_EVENT) { + while (btf_type_is_modifier(t) && !btf_type_is_typedef(t)) + t = btf_type_by_id(btf, t->type); + + if (btf_type_is_typedef(t)) { + tname = btf_name_by_offset(btf, t->name_off); + if (tname && strcmp(tname, "bpf_user_pt_regs_t") == 0) + return 0; + } + } + + /* all other program types don't use typedefs for context type */ + while (btf_type_is_modifier(t)) + t = btf_type_by_id(btf, t->type); + + /* `void *ctx __arg_ctx` is always valid */ + if (btf_type_is_void(t)) + return 0; + + tname = btf_name_by_offset(btf, t->name_off); + if (str_is_empty(tname)) { + bpf_log(log, "arg#%d type doesn't have a name\n", arg); + return -EINVAL; + } + + /* special cases */ + switch (prog_type) { + case BPF_PROG_TYPE_KPROBE: + if (__btf_type_is_struct(t) && strcmp(tname, "pt_regs") == 0) + return 0; + break; + case BPF_PROG_TYPE_PERF_EVENT: + if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct pt_regs) && + __btf_type_is_struct(t) && strcmp(tname, "pt_regs") == 0) + return 0; + if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct user_pt_regs) && + __btf_type_is_struct(t) && strcmp(tname, "user_pt_regs") == 0) + return 0; + if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct user_regs_struct) && + __btf_type_is_struct(t) && strcmp(tname, "user_regs_struct") == 0) + return 0; + break; + case BPF_PROG_TYPE_RAW_TRACEPOINT: + case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE: + /* allow u64* as ctx */ + if (btf_is_int(t) && t->size == 8) + return 0; + break; + case BPF_PROG_TYPE_TRACING: + switch (attach_type) { + case BPF_TRACE_RAW_TP: + /* tp_btf program is TRACING, so need special case here */ + if (__btf_type_is_struct(t) && + strcmp(tname, "bpf_raw_tracepoint_args") == 0) + return 0; + /* allow u64* as ctx */ + if (btf_is_int(t) && t->size == 8) + return 0; + break; + case BPF_TRACE_ITER: + /* allow struct bpf_iter__xxx types only */ + if (__btf_type_is_struct(t) && + strncmp(tname, "bpf_iter__", sizeof("bpf_iter__") - 1) == 0) + return 0; + break; + case BPF_TRACE_FENTRY: + case BPF_TRACE_FEXIT: + case BPF_MODIFY_RETURN: + /* allow u64* as ctx */ + if (btf_is_int(t) && t->size == 8) + return 0; + break; + default: + break; + } + break; + case BPF_PROG_TYPE_LSM: + case BPF_PROG_TYPE_STRUCT_OPS: + /* allow u64* as ctx */ + if (btf_is_int(t) && t->size == 8) + return 0; + break; + case BPF_PROG_TYPE_TRACEPOINT: + case BPF_PROG_TYPE_SYSCALL: + case BPF_PROG_TYPE_EXT: + return 0; /* anything goes */ + default: + break; + } + + ctx_type = find_canonical_prog_ctx_type(prog_type); + if (!ctx_type) { + /* should not happen */ + bpf_log(log, "btf_vmlinux is malformed\n"); + return -EINVAL; + } + + /* resolve typedefs and check that underlying structs are matching as well */ + while (btf_type_is_modifier(ctx_type)) + ctx_type = btf_type_by_id(btf_vmlinux, ctx_type->type); + + /* if program type doesn't have distinctly named struct type for + * context, then __arg_ctx argument can only be `void *`, which we + * already checked above + */ + if (!__btf_type_is_struct(ctx_type)) { + bpf_log(log, "arg#%d should be void pointer\n", arg); + return -EINVAL; + } + + ctx_tname = btf_name_by_offset(btf_vmlinux, ctx_type->name_off); + if (!__btf_type_is_struct(t) || strcmp(ctx_tname, tname) != 0) { + bpf_log(log, "arg#%d should be `struct %s *`\n", arg, ctx_tname); + return -EINVAL; + } + + return 0; +} + static int btf_translate_to_vmlinux(struct bpf_verifier_log *log, struct btf *btf, const struct btf_type *t, @@ -6953,6 +7096,23 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) return -EINVAL; } + for (i = 0; i < nargs; i++) { + const char *tag; + + if (sub->args[i].arg_type != ARG_PTR_TO_CTX) + continue; + + /* check if arg has "arg:ctx" tag */ + t = btf_type_by_id(btf, args[i].type); + tag = btf_find_decl_tag_value(btf, fn_t, i, "arg:"); + if (IS_ERR_OR_NULL(tag) || strcmp(tag, "ctx") != 0) + continue; + + if (btf_validate_prog_ctx_type(log, btf, t, i, prog_type, + prog->expected_attach_type)) + return -EINVAL; + } + sub->arg_cnt = nargs; sub->args_cached = true; From patchwork Thu Jan 18 03:31:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13522353 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 050E45686 for ; Thu, 18 Jan 2024 03:31:59 +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=1705548719; cv=none; b=fbzNYh0S/eAGpKApDDyjWC+rUQ6ZLOJ+1ETv08C6yckqw8qLjqZF0xPk2mjWikxeUtYa6SloV6X21jOEw8gVDeUQ8jP4xl6R/KFM/BzTnSteB333yN01mBH22ZMqZdGueWAXXOXqsmH0Fzz4crh20/bLvlHw4itPQv9Sfa/JvdA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705548719; c=relaxed/simple; bh=XnDNqdOpo01Es/FXqfLWlfzXWRQYAckC3ojy2/oQiCs=; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-Id: X-Mailer:In-Reply-To:References:MIME-Version: Content-Transfer-Encoding; b=Ggk3jwBYvMCBABxAcbMfqAHY/82f/HHa++sm/l4GCSqHzctOLMPjrF69mKvjC1Y1n44/CwsoIZHGbrz6+Ed6H6gEUC3laBDOzek1UpE9Ixf2uUgd3Ivs5zKqN1O3mnVtGM/W6UAt6b8iZy65Dp4C/ahp3NrHUh7EuU/2UamN7NM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=uMeqILeW; 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="uMeqILeW" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A3768C433C7; Thu, 18 Jan 2024 03:31:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705548718; bh=XnDNqdOpo01Es/FXqfLWlfzXWRQYAckC3ojy2/oQiCs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uMeqILeWKErCiyqky4gHo5FrWrSVy064ndYMkgJe06Nth9CUVP8JMVyNt/bOUxHTE WajnqmDzNDKGtuO5JS6bWX9CDB/1VasXJISyqWcaLuD78U41tun9rG/TyU5ZStMtZw +dp2IKIiFIhTKcwuEbHSsmFofHghxw6oR1aJ1gle5mLe10L1B3LypVgB6M8hSeeKfQ K95H41oXze9cmUx9/9c03B0r8TQFT/OtCcAZkKNypTtIzGBc6oqKTEF86Q5X+3tVAo kigqwLOvb70A8Lldt44x5QKrfApGBtV0l8DdtMp8WJcRrDd9X/JcAld1QltiQvWpUt HzbY2RU5y5EQA== 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 v3 bpf 4/5] selftests/bpf: add tests confirming type logic in kernel for __arg_ctx Date: Wed, 17 Jan 2024 19:31:42 -0800 Message-Id: <20240118033143.3384355-5-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org> References: <20240118033143.3384355-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 bunch of global subprogs across variety of program types to validate expected kernel type enforcement logic for __arg_ctx arguments. Signed-off-by: Andrii Nakryiko --- .../bpf/progs/verifier_global_subprogs.c | 164 +++++++++++++++++- 1 file changed, 161 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c index 9eeb2d89cda8..67dddd941891 100644 --- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c +++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c @@ -3,6 +3,7 @@ #include #include +#include #include "bpf_misc.h" #include "xdp_metadata.h" #include "bpf_kfuncs.h" @@ -138,25 +139,182 @@ __weak int subprog_ctx_tag(void *ctx __arg_ctx) return bpf_get_stack(ctx, stack, sizeof(stack), 0); } +__weak int raw_tp_canonical(struct bpf_raw_tracepoint_args *ctx __arg_ctx) +{ + return 0; +} + +__weak int raw_tp_u64_array(u64 *ctx __arg_ctx) +{ + return 0; +} + SEC("?raw_tp") __success __log_level(2) int arg_tag_ctx_raw_tp(void *ctx) { - return subprog_ctx_tag(ctx); + return subprog_ctx_tag(ctx) + raw_tp_canonical(ctx) + raw_tp_u64_array(ctx); +} + +SEC("?raw_tp.w") +__success __log_level(2) +int arg_tag_ctx_raw_tp_writable(void *ctx) +{ + return subprog_ctx_tag(ctx) + raw_tp_canonical(ctx) + raw_tp_u64_array(ctx); +} + +SEC("?tp_btf/sys_enter") +__success __log_level(2) +int arg_tag_ctx_raw_tp_btf(void *ctx) +{ + return subprog_ctx_tag(ctx) + raw_tp_canonical(ctx) + raw_tp_u64_array(ctx); +} + +struct whatever { }; + +__weak int tp_whatever(struct whatever *ctx __arg_ctx) +{ + return 0; } SEC("?tp") __success __log_level(2) int arg_tag_ctx_tp(void *ctx) { - return subprog_ctx_tag(ctx); + return subprog_ctx_tag(ctx) + tp_whatever(ctx); +} + +__weak int kprobe_subprog_pt_regs(struct pt_regs *ctx __arg_ctx) +{ + return 0; +} + +__weak int kprobe_subprog_typedef(bpf_user_pt_regs_t *ctx __arg_ctx) +{ + return 0; } SEC("?kprobe") __success __log_level(2) int arg_tag_ctx_kprobe(void *ctx) { - return subprog_ctx_tag(ctx); + return subprog_ctx_tag(ctx) + + kprobe_subprog_pt_regs(ctx) + + kprobe_subprog_typedef(ctx); +} + +__weak int perf_subprog_regs( +#if defined(bpf_target_riscv) + struct user_regs_struct *ctx __arg_ctx +#elif defined(bpf_target_s390) + /* user_pt_regs typedef is anonymous struct, so only `void *` works */ + void *ctx __arg_ctx +#elif defined(bpf_target_loongarch) || defined(bpf_target_arm64) || defined(bpf_target_powerpc) + struct user_pt_regs *ctx __arg_ctx +#else + struct pt_regs *ctx __arg_ctx +#endif +) +{ + return 0; +} + +__weak int perf_subprog_typedef(bpf_user_pt_regs_t *ctx __arg_ctx) +{ + return 0; +} + +__weak int perf_subprog_canonical(struct bpf_perf_event_data *ctx __arg_ctx) +{ + return 0; +} + +SEC("?perf_event") +__success __log_level(2) +int arg_tag_ctx_perf(void *ctx) +{ + return subprog_ctx_tag(ctx) + + perf_subprog_regs(ctx) + + perf_subprog_typedef(ctx) + + perf_subprog_canonical(ctx); +} + +__weak int iter_subprog_void(void *ctx __arg_ctx) +{ + return 0; +} + +__weak int iter_subprog_typed(struct bpf_iter__task *ctx __arg_ctx) +{ + return 0; +} + +SEC("?iter/task") +__success __log_level(2) +int arg_tag_ctx_iter_task(struct bpf_iter__task *ctx) +{ + return (iter_subprog_void(ctx) + iter_subprog_typed(ctx)) & 1; +} + +__weak int tracing_subprog_void(void *ctx __arg_ctx) +{ + return 0; +} + +__weak int tracing_subprog_u64(u64 *ctx __arg_ctx) +{ + return 0; +} + +int acc; + +SEC("?fentry/" SYS_PREFIX "sys_nanosleep") +__success __log_level(2) +int BPF_PROG(arg_tag_ctx_fentry) +{ + acc += tracing_subprog_void(ctx) + tracing_subprog_u64(ctx); + return 0; +} + +SEC("?fexit/" SYS_PREFIX "sys_nanosleep") +__success __log_level(2) +int BPF_PROG(arg_tag_ctx_fexit) +{ + acc += tracing_subprog_void(ctx) + tracing_subprog_u64(ctx); + return 0; +} + +SEC("?fmod_ret/" SYS_PREFIX "sys_nanosleep") +__success __log_level(2) +int BPF_PROG(arg_tag_ctx_fmod_ret) +{ + return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx); +} + +SEC("?lsm/bpf") +__success __log_level(2) +int BPF_PROG(arg_tag_ctx_lsm) +{ + return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx); +} + +SEC("?struct_ops/test_1") +__success __log_level(2) +int BPF_PROG(arg_tag_ctx_struct_ops) +{ + return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx); +} + +SEC(".struct_ops") +struct bpf_dummy_ops dummy_1 = { + .test_1 = (void *)arg_tag_ctx_struct_ops, +}; + +SEC("?syscall") +__success __log_level(2) +int arg_tag_ctx_syscall(void *ctx) +{ + return tracing_subprog_void(ctx) + tracing_subprog_u64(ctx) + tp_whatever(ctx); } __weak int subprog_dynptr(struct bpf_dynptr *dptr) From patchwork Thu Jan 18 03:31:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrii Nakryiko X-Patchwork-Id: 13522354 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 2F8007497 for ; Thu, 18 Jan 2024 03:32: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=1705548722; cv=none; b=elVMc+rCaeq3L3JdxXLlTJWmHpD5w8f/OCAVizWJyNIz2UrkMujzepraXJkSd1GPvJ/MRKRMoVDnuykfMVMgSlUMNv8h0Movy34zyZlfQQfZtQopQpXDHr67MsN89Wh1D+AlQqKbaiB7qfMdSHKVlAq2ll58M8A+MgNQe07w678= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705548722; c=relaxed/simple; bh=oPWDFSW8tax/36j2efysjzZL6ujs01Pc7gcmMx6xyds=; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-Id: X-Mailer:In-Reply-To:References:MIME-Version: Content-Transfer-Encoding; b=JnTNV+OoNBbuB2xvjZwxV6IJi7TDN3a2EA8V1fimqQ8nnWoL2ZLLZhPnI7gRp0wH7G1RJc5u4oE66F/KXcKOi1hZWy5t6/08brLyqaVY18xZOxtWqobLPQBYGuvTo0HDkbGEvEvk2Z8wycrEiXtorDzN8+/mUhMUqzdU0HEoakQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=YqtUqQFS; 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="YqtUqQFS" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D2A4DC433F1; Thu, 18 Jan 2024 03:32:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705548722; bh=oPWDFSW8tax/36j2efysjzZL6ujs01Pc7gcmMx6xyds=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YqtUqQFSmUl9p5bVZBcRLSdN+WJkzvA+1yqkktKxh1tJ+5BBBEFzutM6OPTdBZUAV h6hJjcSYbhSSiqdt0E2AcwJFpxPKHmfFxHwm6Wm0196h2B40ybbcNQRY3CxQf19e2q ECEhz2p96dBenuyUqAbn9rqCTDmOIE1e+XDuXxLh8WJL/o3srY30qW3MQ0Mk/XbSY2 euu0stA+h0hEhBmqcRoi2zoGfzDKLbgY8i0RTAEe8NTzM8uU3ic9Sst6XY5ASrDjkH dJDKW9Hj3/b3BGiQ5WON+MxD5staF95kPZ1UPpIGT6+Rdi7z3FO5CqcwRAuLHe5ay9 /Zoo1Xe3VEJJw== 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 v3 bpf 5/5] libbpf: warn on unexpected __arg_ctx type when rewriting BTF Date: Wed, 17 Jan 2024 19:31:43 -0800 Message-Id: <20240118033143.3384355-6-andrii@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240118033143.3384355-1-andrii@kernel.org> References: <20240118033143.3384355-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 On kernel that don't support arg:ctx tag, before adjusting global subprog BTF information to match kernel's expected canonical type names, make sure that types used by user are meaningful, and if not, warn and don't do BTF adjustments. This is similar to checks that kernel performs, but narrower in scope, as only a small subset of BPF program types can be accommodated by libbpf using canonical type names. Libbpf unconditionally allows `struct pt_regs *` for perf_event program types, unlike kernel, which supports that conditionally on architecture. This is done to keep things simple and not cause unnecessary false positives. This seems like a minor and harmless deviation, which in real-world programs will be caught by kernels with arg:ctx tag support anyways. So KISS principle. This logic is hard to test (especially on latest kernels), so manual testing was performed instead. Libbpf emitted the following warning for perf_event program with wrong context argument type: libbpf: prog 'arg_tag_ctx_perf': subprog 'subprog_ctx_tag' arg#0 is expected to be of `struct bpf_perf_event_data *` type Signed-off-by: Andrii Nakryiko --- tools/lib/bpf/libbpf.c | 75 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 61db92189517..afd09571c482 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -6695,6 +6695,67 @@ static struct { /* all other program types don't have "named" context structs */ }; +static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_program *prog, + const char *subprog_name, int arg_idx, + int arg_type_id, const char *ctx_name) +{ + const struct btf_type *t; + const char *tname; + + /* check if existing parameter already matches verifier expectations */ + t = skip_mods_and_typedefs(btf, arg_type_id, NULL); + if (!btf_is_ptr(t)) + goto out_warn; + + /* typedef bpf_user_pt_regs_t is a special PITA case, valid for kprobe + * and perf_event programs, so check this case early on and forget + * about it for subsequent checks + */ + while (btf_is_mod(t)) + t = btf__type_by_id(btf, t->type); + if (btf_is_typedef(t) && + (prog->type == BPF_PROG_TYPE_KPROBE || prog->type == BPF_PROG_TYPE_PERF_EVENT)) { + tname = btf__str_by_offset(btf, t->name_off) ?: ""; + if (strcmp(tname, "bpf_user_pt_regs_t") == 0) + return false; /* canonical type for kprobe/perf_event */ + } + + /* now we can ignore typedefs moving forward */ + t = skip_mods_and_typedefs(btf, t->type, NULL); + + /* if it's `void *`, definitely fix up BTF info */ + if (btf_is_void(t)) + return true; + + /* if it's already proper canonical type, no need to fix up */ + tname = btf__str_by_offset(btf, t->name_off) ?: ""; + if (btf_is_struct(t) && strcmp(tname, ctx_name) == 0) + return false; + + /* special cases */ + switch (prog->type) { + case BPF_PROG_TYPE_KPROBE: + case BPF_PROG_TYPE_PERF_EVENT: + /* `struct pt_regs *` is expected, but we need to fix up */ + if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0) + return true; + break; + case BPF_PROG_TYPE_RAW_TRACEPOINT: + case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE: + /* allow u64* as ctx */ + if (btf_is_int(t) && t->size == 8) + return true; + break; + default: + break; + } + +out_warn: + pr_warn("prog '%s': subprog '%s' arg#%d is expected to be of `struct %s *` type\n", + prog->name, subprog_name, arg_idx, ctx_name); + return false; +} + static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog) { int fn_id, fn_proto_id, ret_type_id, orig_proto_id; @@ -6829,7 +6890,7 @@ static int probe_kern_arg_ctx_tag(void) */ static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog) { - const char *ctx_name = NULL, *ctx_tag = "arg:ctx"; + const char *ctx_name = NULL, *ctx_tag = "arg:ctx", *fn_name; struct bpf_func_info_min *func_rec; struct btf_type *fn_t, *fn_proto_t; struct btf *btf = obj->btf; @@ -6909,15 +6970,11 @@ static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_progra if (arg_idx < 0 || arg_idx >= arg_cnt) continue; - /* check if existing parameter already matches verifier expectations */ + /* check if we should fix up argument type */ p = &btf_params(fn_proto_t)[arg_idx]; - t = skip_mods_and_typedefs(btf, p->type, NULL); - if (btf_is_ptr(t) && - (t = skip_mods_and_typedefs(btf, t->type, NULL)) && - btf_is_struct(t) && - strcmp(btf__str_by_offset(btf, t->name_off), ctx_name) == 0) { - continue; /* no need for fix up */ - } + fn_name = btf__str_by_offset(btf, fn_t->name_off) ?: ""; + if (!need_func_arg_type_fixup(btf, prog, fn_name, arg_idx, p->type, ctx_name)) + continue; /* clone fn/fn_proto, unless we already did it for another arg */ if (func_rec->type_id == orig_fn_id) {