From patchwork Thu Sep 5 07:56:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philo Lu X-Patchwork-Id: 13791860 X-Patchwork-Delegate: bpf@iogearbox.net Received: from out30-132.freemail.mail.aliyun.com (out30-132.freemail.mail.aliyun.com [115.124.30.132]) (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 5CD1319340D; Thu, 5 Sep 2024 07:56:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.132 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522997; cv=none; b=UnY+kKHIqDOcoJcD0ggeWGJozfzEgCxGl2CFz1BfsS/YBbyHCljmuxmXVdRLKFQWa74qyi8aTYUAuBP5+8K2DdH0/9XMPBIOMUAHP29C3kpOiC/WVbj6PrWrw0fLoVLbiRj/oiUKiRfvae1Jm8dA5N4B5lH3MHIZ2WbtvXa23Dw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522997; c=relaxed/simple; bh=67C+ySKUMFOl/Vw/EWHL5BU7fz6lIct5OzmIokYnQIQ=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=Yk3T06kUhWX9e0aLjWc7YM0RZqkzu1ZLQA//De9Ba7IPXBCVFlEc57QLIN7YJuI/J6FuigDM1x7R1xpfZoNRepgIJcSPIEFWYiayV3VbDnCvbgbDQamnlerSMCuvKjbsU9HvJSVTeWT0TF4X7oCyfj66It6bPsuFbR9jAnfSyRA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=doEQQoTG; arc=none smtp.client-ip=115.124.30.132 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="doEQQoTG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725522986; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=XIpXmKG/dvqtkO380OYf1vfqjzkl8IpnbYNB+J/73MQ=; b=doEQQoTG/kdjL2Y7oLNSv/Y5ZA0ZtaZJs/d8Kl+YgCImoiCicQ+F1ePQi8tZ9J9Y4dSwb+EEY9iZtUu0rZymLtLvf/4pIsZHaGJTuh3vPHEFknPKVQlo4J2BdXQ2mHRBuF7gFsTJ+uC0KJmQhAwSkOzXUjizYAAQAYeeyTwYxss= Received: from localhost(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0WEKrrCP_1725522983) by smtp.aliyun-inc.com; Thu, 05 Sep 2024 15:56:24 +0800 From: Philo Lu To: bpf@vger.kernel.org Cc: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, juntong.deng@outlook.com, jrife@google.com, alan.maguire@oracle.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, vmalik@redhat.com, cupertino.miranda@oracle.com, mattbobrowski@google.com, xuanzhuo@linux.alibaba.com, netdev@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH bpf-next v2 1/5] bpf: Support __nullable argument suffix for tp_btf Date: Thu, 5 Sep 2024 15:56:18 +0800 Message-Id: <20240905075622.66819-2-lulie@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240905075622.66819-1-lulie@linux.alibaba.com> References: <20240905075622.66819-1-lulie@linux.alibaba.com> 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 Pointers passed to tp_btf were trusted to be valid, but some tracepoints do take NULL pointer as input, such as trace_tcp_send_reset(). Then the invalid memory access cannot be detected by verifier. This patch fix it by add a suffix "__nullable" to the unreliable argument. The suffix is shown in btf, and PTR_MAYBE_NULL will be added to nullable arguments. Then users must check the pointer before use it. A problem here is that we use "btf_trace_##call" to search func_proto. As it is a typedef, argument names as well as the suffix are not recorded. To solve this, I use bpf_raw_event_map to find "__bpf_trace##template" from "btf_trace_##call", and then we can see the suffix. Suggested-by: Alexei Starovoitov Signed-off-by: Philo Lu --- kernel/bpf/btf.c | 13 +++++++++++++ kernel/bpf/verifier.c | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 1e29281653c62..157f5e1247c81 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6385,6 +6385,16 @@ static bool prog_args_trusted(const struct bpf_prog *prog) } } +static bool prog_arg_maybe_null(const struct bpf_prog *prog, const struct btf *btf, + const struct btf_param *arg) +{ + if (prog->type != BPF_PROG_TYPE_TRACING || + prog->expected_attach_type != BPF_TRACE_RAW_TP) + return false; + + return btf_param_match_suffix(btf, arg, "__nullable"); +} + int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto, u32 arg_no) { @@ -6554,6 +6564,9 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, if (prog_args_trusted(prog)) info->reg_type |= PTR_TRUSTED; + if (prog_arg_maybe_null(prog, btf, &args[arg])) + info->reg_type |= PTR_MAYBE_NULL; + if (tgt_prog) { enum bpf_prog_type tgt_type; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 217eb0eafa2a6..9ee68ed915dfe 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "disasm.h" @@ -21780,6 +21781,8 @@ static int check_non_sleepable_error_inject(u32 btf_id) return btf_id_set_contains(&btf_non_sleepable_error_inject, btf_id); } +#define BTF_MAX_NAME_SIZE 128 + int bpf_check_attach_target(struct bpf_verifier_log *log, const struct bpf_prog *prog, const struct bpf_prog *tgt_prog, @@ -21788,6 +21791,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, { bool prog_extension = prog->type == BPF_PROG_TYPE_EXT; bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING; + char trace_symbol[BTF_MAX_NAME_SIZE]; + const struct bpf_raw_event_map *btp; const char prefix[] = "btf_trace_"; int ret = 0, subprog = -1, i; const struct btf_type *t; @@ -21795,6 +21800,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, const char *tname; struct btf *btf; long addr = 0; + char *fname; struct module *mod = NULL; if (!btf_id) { @@ -21923,10 +21929,34 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, return -EINVAL; } tname += sizeof(prefix) - 1; - t = btf_type_by_id(btf, t->type); - if (!btf_type_is_ptr(t)) - /* should never happen in valid vmlinux build */ + + /* The func_proto of "btf_trace_##tname" is generated from typedef without argument + * names. Thus using bpf_raw_event_map to get argument names. For module, the module + * name is printed in "%ps" after the template function name, so use strsep to cut + * it off. + */ + btp = bpf_get_raw_tracepoint(tname); + if (!btp) return -EINVAL; + sprintf(trace_symbol, "%ps", btp->bpf_func); + fname = trace_symbol; + fname = strsep(&fname, " "); + + ret = btf_find_by_name_kind(btf, fname, BTF_KIND_FUNC); + if (ret < 0) { + bpf_log(log, "Cannot find btf of template %s, fall back to %s%s.\n", + fname, prefix, tname); + t = btf_type_by_id(btf, t->type); + if (!btf_type_is_ptr(t)) + /* should never happen in valid vmlinux build */ + return -EINVAL; + } else { + t = btf_type_by_id(btf, ret); + if (!btf_type_is_func(t)) + /* should never happen in valid vmlinux build */ + return -EINVAL; + } + t = btf_type_by_id(btf, t->type); if (!btf_type_is_func_proto(t)) /* should never happen in valid vmlinux build */ From patchwork Thu Sep 5 07:56:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philo Lu X-Patchwork-Id: 13791856 X-Patchwork-Delegate: bpf@iogearbox.net Received: from out30-131.freemail.mail.aliyun.com (out30-131.freemail.mail.aliyun.com [115.124.30.131]) (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 3A4EE19340B; Thu, 5 Sep 2024 07:56:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.131 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; cv=none; b=tPk8uwroS0SEKk6cbknquvBy6s2xm+WmuRAUqOqfsYFRJXDkgjlk/Chv963EIO2cWszDt1GZXRMrLIGQbLAKo7WQ49Ww2EXXzA++7RzvA+tzzgxbTBuKEcwvH3ZcIrJBILAoP/XQ+pSJlViW4N9WuFAVhD3ZMzBsWGd7H6wxvz4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; c=relaxed/simple; bh=oJ5g20RlH+r1YZadkqhgMik60Qt05kzPuIVi50Voc/c=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=I9qJUKjULd7w/avGWQ7nDVs8O9vzmLp694nIQUrtw4nYN4O6iqbPcLWpm4ZRlLShIDIaSfmBIISNlogbdS5G86svE8mG7Cn2UBPJ264VJ8YGqUCI/+CNvc4sMODK0jonf9sdMk9KMLjH006z5vIKD90KGT7oz/QzLWBHJuJmkt8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=InjgB6b3; arc=none smtp.client-ip=115.124.30.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="InjgB6b3" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725522989; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=JKqdxuX4Wa1ORQ7kxclAATsobclvetQtCCCwpC0uu+E=; b=InjgB6b3i6+KIKo+2wfWdXQiCn67ICIXq2MiYWR77x8ZKMKVEthyPLhtXQwMF9MNQEBg/NdzOKiII67GMLkzuRuXJDdP+obTZNtJbCl/bt15w45MrOnocCTnpIHDhwtWwNXRvYuzXOT4Q+g3U9AgDlTwBGqdCwRi0qOYi0Jqqos= Received: from localhost(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0WEKuTCT_1725522985) by smtp.aliyun-inc.com; Thu, 05 Sep 2024 15:56:26 +0800 From: Philo Lu To: bpf@vger.kernel.org Cc: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, juntong.deng@outlook.com, jrife@google.com, alan.maguire@oracle.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, vmalik@redhat.com, cupertino.miranda@oracle.com, mattbobrowski@google.com, xuanzhuo@linux.alibaba.com, netdev@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH bpf-next v2 2/5] selftests/bpf: Add test for __nullable suffix in tp_btf Date: Thu, 5 Sep 2024 15:56:19 +0800 Message-Id: <20240905075622.66819-3-lulie@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240905075622.66819-1-lulie@linux.alibaba.com> References: <20240905075622.66819-1-lulie@linux.alibaba.com> 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 tracepoint with __nullable suffix in bpf_testmod, and add a failure load case: $./test_progs -t "module_attach" #173/1 module_attach/handle_tp_btf_nullable_bare:OK #173 module_attach:OK Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Philo Lu --- .../bpf/bpf_testmod/bpf_testmod-events.h | 6 ++++++ .../selftests/bpf/bpf_testmod/bpf_testmod.c | 2 ++ .../selftests/bpf/prog_tests/module_attach.c | 14 +++++++++++++- .../bpf/progs/test_module_attach_fail.c | 16 ++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/test_module_attach_fail.c diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h index 11ee801e75e7e..6c3b4d4f173ac 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h @@ -34,6 +34,12 @@ DECLARE_TRACE(bpf_testmod_test_write_bare, TP_ARGS(task, ctx) ); +/* Used in bpf_testmod_test_read() to test __nullable suffix */ +DECLARE_TRACE(bpf_testmod_test_nullable_bare, + TP_PROTO(struct bpf_testmod_test_read_ctx *ctx__nullable), + TP_ARGS(ctx__nullable) +); + #undef BPF_TESTMOD_DECLARE_TRACE #ifdef DECLARE_TRACE_WRITABLE #define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \ diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c index c73d04bc9e9de..9649e7f09fc90 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c @@ -394,6 +394,8 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj, if (bpf_testmod_loop_test(101) > 100) trace_bpf_testmod_test_read(current, &ctx); + trace_bpf_testmod_test_nullable_bare(NULL); + /* Magic number to enable writable tp */ if (len == 64) { struct bpf_testmod_test_writable_ctx writable = { diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c index 6d391d95f96e0..961d8577d6fab 100644 --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c @@ -4,6 +4,7 @@ #include #include #include "test_module_attach.skel.h" +#include "test_module_attach_fail.skel.h" #include "testing_helpers.h" static int duration; @@ -33,7 +34,7 @@ static int trigger_module_test_writable(int *val) return 0; } -void test_module_attach(void) +static void module_attach_succ(void) { const int READ_SZ = 456; const int WRITE_SZ = 457; @@ -115,3 +116,14 @@ void test_module_attach(void) cleanup: test_module_attach__destroy(skel); } + +static void module_attach_fail(void) +{ + RUN_TESTS(test_module_attach_fail); +} + +void test_module_attach(void) +{ + module_attach_succ(); + module_attach_fail(); +} diff --git a/tools/testing/selftests/bpf/progs/test_module_attach_fail.c b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c new file mode 100644 index 0000000000000..0f848d8f2f5e8 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "vmlinux.h" +#include +#include +#include "../bpf_testmod/bpf_testmod.h" +#include "bpf_misc.h" + +SEC("tp_btf/bpf_testmod_test_nullable_bare") +__failure __msg("invalid mem access") +int BPF_PROG(handle_tp_btf_nullable_bare, struct bpf_testmod_test_read_ctx *nullable_ctx) +{ + return nullable_ctx->len; +} + +char _license[] SEC("license") = "GPL"; From patchwork Thu Sep 5 07:56:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philo Lu X-Patchwork-Id: 13791857 X-Patchwork-Delegate: bpf@iogearbox.net Received: from out30-131.freemail.mail.aliyun.com (out30-131.freemail.mail.aliyun.com [115.124.30.131]) (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 3A4A0BE4A; Thu, 5 Sep 2024 07:56:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.131 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; cv=none; b=qtg/oH/oLXg9Q8SJZ5+vqUIEK5GJrIXbUW3nM6XA4WpF0EyFl9t2UGNztJjCBft6AjrtJS+YKrcByRXDT/4tDzcXkBg5/1JBpd8B/Ypuwmo8Ci/+7/k6uPf1yGhHF4EyScS8asJwmV9oQbx4Zzv4t8jiBy7nmCSkwGnwve/CZyw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; c=relaxed/simple; bh=K0e6mQkeHBgMEA2rErVI/IFaj39YX4medY8VKPXIqoo=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=aC5uOvwPJ/XbhUdOEG8SvpqbMWbNKPk0mNBf21qi/KAnBy5wbex6FH8OfOitH20SX1sxDgtbv5opR/2s2g+PFJeN6LcWHPev6QWEcz2DD2z9sn09xl5wz8bNbKqcvxtsp94x5BrPQxgkqu0RI31GkYKkAhS0TCCecJe+vXLOdgo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=UQlpqKa9; arc=none smtp.client-ip=115.124.30.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="UQlpqKa9" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725522989; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=NomjT5FnOgrVw/o0cAzY3uTGKhJT56vZeS0KR7kFDPo=; b=UQlpqKa9/TjNKv1/sEacSBmPjVyyJfJJh5JtnJbS70V+NDEdB0Y7/g71nq+A/hUg+pJpnPdhGQAKvcLeHY6Ln7jU49VgSGf1MUERpARXIBJ/09531FGSCzj6tZjwzuunvoW7AFaP3ISPS5+ucbODoUljboBON0k/NNXOS0gk1LA= Received: from localhost(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0WEKkPf-_1725522986) by smtp.aliyun-inc.com; Thu, 05 Sep 2024 15:56:27 +0800 From: Philo Lu To: bpf@vger.kernel.org Cc: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, juntong.deng@outlook.com, jrife@google.com, alan.maguire@oracle.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, vmalik@redhat.com, cupertino.miranda@oracle.com, mattbobrowski@google.com, xuanzhuo@linux.alibaba.com, netdev@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH bpf-next v2 3/5] tcp: Use skb__nullable in trace_tcp_send_reset Date: Thu, 5 Sep 2024 15:56:20 +0800 Message-Id: <20240905075622.66819-4-lulie@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240905075622.66819-1-lulie@linux.alibaba.com> References: <20240905075622.66819-1-lulie@linux.alibaba.com> 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 Replace skb with skb__nullable as the argument name. The suffix tells bpf verifier through btf that the arg could be NULL and should be checked in tp_btf prog. For now, this is the only nullable argument in tcp tracepoints. Signed-off-by: Philo Lu Acked-by: Jakub Kicinski --- include/trace/events/tcp.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h index 1c8bd8e186b89..a27c4b619dffd 100644 --- a/include/trace/events/tcp.h +++ b/include/trace/events/tcp.h @@ -91,10 +91,10 @@ DEFINE_RST_REASON(FN, FN) TRACE_EVENT(tcp_send_reset, TP_PROTO(const struct sock *sk, - const struct sk_buff *skb, + const struct sk_buff *skb__nullable, const enum sk_rst_reason reason), - TP_ARGS(sk, skb, reason), + TP_ARGS(sk, skb__nullable, reason), TP_STRUCT__entry( __field(const void *, skbaddr) @@ -106,7 +106,7 @@ TRACE_EVENT(tcp_send_reset, ), TP_fast_assign( - __entry->skbaddr = skb; + __entry->skbaddr = skb__nullable; __entry->skaddr = sk; /* Zero means unknown state. */ __entry->state = sk ? sk->sk_state : 0; @@ -118,13 +118,13 @@ TRACE_EVENT(tcp_send_reset, const struct inet_sock *inet = inet_sk(sk); TP_STORE_ADDR_PORTS(__entry, inet, sk); - } else if (skb) { - const struct tcphdr *th = (const struct tcphdr *)skb->data; + } else if (skb__nullable) { + const struct tcphdr *th = (const struct tcphdr *)skb__nullable->data; /* * We should reverse the 4-tuple of skb, so later * it can print the right flow direction of rst. */ - TP_STORE_ADDR_PORTS_SKB(skb, th, entry->daddr, entry->saddr); + TP_STORE_ADDR_PORTS_SKB(skb__nullable, th, entry->daddr, entry->saddr); } __entry->reason = reason; ), From patchwork Thu Sep 5 07:56:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philo Lu X-Patchwork-Id: 13791858 X-Patchwork-Delegate: bpf@iogearbox.net Received: from out30-97.freemail.mail.aliyun.com (out30-97.freemail.mail.aliyun.com [115.124.30.97]) (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 5EB58193422; Thu, 5 Sep 2024 07:56:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.97 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; cv=none; b=FyNzgx3fAx6Q7qYJFzHiyITT2Nwf60Rgm9fjrpPad9swV3pNJkYdd5p6raHaTfLzX+ZmmIS3TB+Pw6DJYky+fYc0cKQfztLjsGiUI1WMxA54nL+Utjm6BNYtB0IKsYVxcv4jQaCfNNqC8U0seS/5IGAOjvnu0xeYXa6vtzvTE9o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725522994; c=relaxed/simple; bh=9nX5BEqo/bJuVUV4T906+zK42dyZFMxqoxbNcOlRtAU=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=drijcX7jjSCbfnr+4yDm4RQDvphP0AFhmBXfXSHmlzYWv9tFADbu0NxvxRb4d2ZWCU//hG1ZqPXQsGU908Bkx2lrCRGn0NKg9GK9FQ/CewGKyldMnASu2o7FSjBBpoMfSp/5Xbvl4isgjl3U9gCsnpoZM8KrELuhQC2LcRbvp9Y= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=SM4dkg2w; arc=none smtp.client-ip=115.124.30.97 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="SM4dkg2w" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725522990; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=heg5ujh9995FGb5Bxbbr9bVFNtt01S491isl+3eyVds=; b=SM4dkg2wDBVCtiM81sxkeUyOD1h+8jkHwb4OF48R0mRF7dCAjNGwFKrmb3MczM2S3eSBy5aQfCrx0Mc7peP4M1XcKd/N4lTFYHs5ULndurGNcY0/SNfNLbALwaCXCKe9BioxGXZ7yxHNU5SNwpuS2vUVnFfrLpNAK+aQ/uncZ44= Received: from localhost(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0WEKuTDv_1725522987) by smtp.aliyun-inc.com; Thu, 05 Sep 2024 15:56:28 +0800 From: Philo Lu To: bpf@vger.kernel.org Cc: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, juntong.deng@outlook.com, jrife@google.com, alan.maguire@oracle.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, vmalik@redhat.com, cupertino.miranda@oracle.com, mattbobrowski@google.com, xuanzhuo@linux.alibaba.com, netdev@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH bpf-next v2 4/5] bpf: Allow bpf_dynptr_from_skb() for tp_btf Date: Thu, 5 Sep 2024 15:56:21 +0800 Message-Id: <20240905075622.66819-5-lulie@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240905075622.66819-1-lulie@linux.alibaba.com> References: <20240905075622.66819-1-lulie@linux.alibaba.com> 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 Making tp_btf able to use bpf_dynptr_from_skb(), which is useful for skb parsing, especially for non-linear paged skb data. This is achieved by adding KF_TRUSTED_ARGS flag to bpf_dynptr_from_skb and registering it for TRACING progs. With KF_TRUSTED_ARGS, args from fentry/fexit are excluded, so that unsafe progs like fexit/__kfree_skb are not allowed. We also need the skb dynptr to be read-only in tp_btf. Because may_access_direct_pkt_data() returns false by default when checking bpf_dynptr_from_skb, there is no need to add BPF_PROG_TYPE_TRACING to it explicitly. Suggested-by: Martin KaFai Lau Signed-off-by: Philo Lu Acked-by: Martin KaFai Lau --- net/core/filter.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/core/filter.c b/net/core/filter.c index 6d4fa9198b652..4c01f4756ddb5 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -12074,7 +12074,7 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, } BTF_KFUNCS_START(bpf_kfunc_check_set_skb) -BTF_ID_FLAGS(func, bpf_dynptr_from_skb) +BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS) BTF_KFUNCS_END(bpf_kfunc_check_set_skb) BTF_KFUNCS_START(bpf_kfunc_check_set_xdp) @@ -12123,6 +12123,7 @@ static int __init bpf_kfunc_init(void) ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_kfunc_set_skb); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, &bpf_kfunc_set_sock_addr); From patchwork Thu Sep 5 07:56:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philo Lu X-Patchwork-Id: 13791861 X-Patchwork-Delegate: bpf@iogearbox.net Received: from out30-132.freemail.mail.aliyun.com (out30-132.freemail.mail.aliyun.com [115.124.30.132]) (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 006C219340D; Thu, 5 Sep 2024 07:56:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.132 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725523001; cv=none; b=bLjtGaLqiTx0Es9menznPfVvT5Ux6Il/W80yH331szXH47lbxlfu/12whxuB3lKWKJUrm4j3NeP9Vcg/hiorAWZQ0icaZN5hqFuT8ZgNu/1+lJvEABkeDGh5mBhFhVrosTEjqkmnvD2Ug3OWsXHqdZyZSVonQqbbEtcyzZLu69k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725523001; c=relaxed/simple; bh=fQA4ZmvJjUsqFOFX7s+On/lT0lNIldNAFmE9GyYmXH4=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=PrHW+xpG5tq+WOh8ZD3G/r56TTum+cPOEvaPNUcs89klvX9+dm5thRu4uB4IPGGUabp7kfznbDzVI5lAijFSVUKPZNMGf4FXjs55kDq/n9ZGuPHWQ/owjXis7BR7YBnQ3KSZVo/8+BfoMsEC1ToHBu0oIO7PvHtCutY+p+paBxQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=fRtXpEmo; arc=none smtp.client-ip=115.124.30.132 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="fRtXpEmo" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725522991; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=kt0hxrOo2gJqVz1HQLnyjblcCFmCzmyTJPG3om2eI2M=; b=fRtXpEmoGjVhmbYQKmzy6Af/gJJrQzZbC87YVy4aZkhlcQQcm1oLEiK99B/K+Fp07jzLs0uG/ZIR1TgYOjL1MTyVK1aq/hMXHVrDOSWIUcaS1dQLPlf9vk5WpJ48XvmZhMhPteOW9PyF/0j5tfTGdjqHLQTHdCk3+OE80w2zxGo= Received: from localhost(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0WEKkPgj_1725522989) by smtp.aliyun-inc.com; Thu, 05 Sep 2024 15:56:29 +0800 From: Philo Lu To: bpf@vger.kernel.org Cc: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, mathieu.desnoyers@efficios.com, martin.lau@linux.dev, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org, davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, mykolal@fb.com, shuah@kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, thinker.li@gmail.com, juntong.deng@outlook.com, jrife@google.com, alan.maguire@oracle.com, davemarchevsky@fb.com, dxu@dxuuu.xyz, vmalik@redhat.com, cupertino.miranda@oracle.com, mattbobrowski@google.com, xuanzhuo@linux.alibaba.com, netdev@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH bpf-next v2 5/5] selftests/bpf: Expand skb dynptr selftests for tp_btf Date: Thu, 5 Sep 2024 15:56:22 +0800 Message-Id: <20240905075622.66819-6-lulie@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240905075622.66819-1-lulie@linux.alibaba.com> References: <20240905075622.66819-1-lulie@linux.alibaba.com> 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 3 test cases for skb dynptr used in tp_btf: - test_dynptr_skb_tp_btf: use skb dynptr in tp_btf and make sure it is read-only. - skb_invalid_ctx_fentry/skb_invalid_ctx_fexit: bpf_dynptr_from_skb should fail in fentry/fexit. In test_dynptr_skb_tp_btf, to trigger the tracepoint in kfree_skb, test_pkt_access is used for its test_run, as in kfree_skb.c. Because the test process is different from others, a new setup type is defined, i.e., SETUP_SKB_PROG_TP. The result is like: $ ./test_progs -t 'dynptr/test_dynptr_skb_tp_btf' #84/14 dynptr/test_dynptr_skb_tp_btf:OK #84 dynptr:OK #127 kfunc_dynptr_param:OK Summary: 2/1 PASSED, 0 SKIPPED, 0 FAILED $ ./test_progs -t 'dynptr/skb_invalid_ctx_f' #84/85 dynptr/skb_invalid_ctx_fentry:OK #84/86 dynptr/skb_invalid_ctx_fexit:OK #84 dynptr:OK #127 kfunc_dynptr_param:OK Summary: 2/2 PASSED, 0 SKIPPED, 0 FAILED Also fix two coding style nits (change spaces to tabs). Signed-off-by: Philo Lu --- .../testing/selftests/bpf/prog_tests/dynptr.c | 36 +++++++++++++++++-- .../testing/selftests/bpf/progs/dynptr_fail.c | 25 +++++++++++++ .../selftests/bpf/progs/dynptr_success.c | 23 ++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index 7cfac53c0d58d..ba40be8b1c4ef 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -9,6 +9,7 @@ enum test_setup_type { SETUP_SYSCALL_SLEEP, SETUP_SKB_PROG, + SETUP_SKB_PROG_TP, }; static struct { @@ -28,6 +29,7 @@ static struct { {"test_dynptr_clone", SETUP_SKB_PROG}, {"test_dynptr_skb_no_buff", SETUP_SKB_PROG}, {"test_dynptr_skb_strcmp", SETUP_SKB_PROG}, + {"test_dynptr_skb_tp_btf", SETUP_SKB_PROG_TP}, }; static void verify_success(const char *prog_name, enum test_setup_type setup_type) @@ -35,7 +37,7 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ struct dynptr_success *skel; struct bpf_program *prog; struct bpf_link *link; - int err; + int err; skel = dynptr_success__open(); if (!ASSERT_OK_PTR(skel, "dynptr_success__open")) @@ -47,7 +49,7 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) goto cleanup; - bpf_program__set_autoload(prog, true); + bpf_program__set_autoload(prog, true); err = dynptr_success__load(skel); if (!ASSERT_OK(err, "dynptr_success__load")) @@ -87,6 +89,36 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ break; } + case SETUP_SKB_PROG_TP: + { + struct __sk_buff skb = {}; + struct bpf_object *obj; + int aux_prog_fd; + + /* Just use its test_run to trigger kfree_skb tracepoint */ + err = bpf_prog_test_load("./test_pkt_access.bpf.o", BPF_PROG_TYPE_SCHED_CLS, + &obj, &aux_prog_fd); + if (!ASSERT_OK(err, "prog_load sched cls")) + goto cleanup; + + LIBBPF_OPTS(bpf_test_run_opts, topts, + .data_in = &pkt_v4, + .data_size_in = sizeof(pkt_v4), + .ctx_in = &skb, + .ctx_size_in = sizeof(skb), + ); + + link = bpf_program__attach(prog); + if (!ASSERT_OK_PTR(link, "bpf_program__attach")) + goto cleanup; + + err = bpf_prog_test_run_opts(aux_prog_fd, &topts); + + if (!ASSERT_OK(err, "test_run")) + goto cleanup; + + break; + } } ASSERT_EQ(skel->bss->err, 0, "err"); diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index 68b8c6eca5083..8f36c9de75915 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "bpf_misc.h" #include "bpf_kfuncs.h" @@ -1254,6 +1255,30 @@ int skb_invalid_ctx(void *ctx) return 0; } +SEC("fentry/skb_tx_error") +__failure __msg("must be referenced or trusted") +int BPF_PROG(skb_invalid_ctx_fentry, void *skb) +{ + struct bpf_dynptr ptr; + + /* this should fail */ + bpf_dynptr_from_skb(skb, 0, &ptr); + + return 0; +} + +SEC("fexit/skb_tx_error") +__failure __msg("must be referenced or trusted") +int BPF_PROG(skb_invalid_ctx_fexit, void *skb) +{ + struct bpf_dynptr ptr; + + /* this should fail */ + bpf_dynptr_from_skb(skb, 0, &ptr); + + return 0; +} + /* Reject writes to dynptr slot for uninit arg */ SEC("?raw_tp") __failure __msg("potential write to dynptr at off=-16") diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c index 5985920d162e7..bfcc85686cf04 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_success.c +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c @@ -5,6 +5,7 @@ #include #include #include +#include #include "bpf_misc.h" #include "bpf_kfuncs.h" #include "errno.h" @@ -544,3 +545,25 @@ int test_dynptr_skb_strcmp(struct __sk_buff *skb) return 1; } + +SEC("tp_btf/kfree_skb") +int BPF_PROG(test_dynptr_skb_tp_btf, void *skb, void *location) +{ + __u8 write_data[2] = {1, 2}; + struct bpf_dynptr ptr; + int ret; + + if (bpf_dynptr_from_skb(skb, 0, &ptr)) { + err = 1; + return 1; + } + + /* since tp_btf skbs are read only, writes should fail */ + ret = bpf_dynptr_write(&ptr, 0, write_data, sizeof(write_data), 0); + if (ret != -EINVAL) { + err = 2; + return 1; + } + + return 1; +}