All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Yonghong Song <yonghong.song@linux.dev>,
	David Vernet <void@manifault.com>
Subject: [PATCH bpf-next v2 10/14] bpf: Disallow extensions to exception callbacks
Date: Wed,  9 Aug 2023 17:11:12 +0530	[thread overview]
Message-ID: <20230809114116.3216687-11-memxor@gmail.com> (raw)
In-Reply-To: <20230809114116.3216687-1-memxor@gmail.com>

During testing, it was discovered that extensions to exception callbacks
had no checks, upon running a testcase, the kernel ended up running off
the end of a program having final call as bpf_throw, and hitting int3
instructions.

The reason is that while the default exception callback would have reset
the stack frame to return back to the main program's caller, the
replacing extension program will simply return back to bpf_throw, which
will instead return back to the program and the program will continue
execution, now in an undefined state where anything could happen.

The way to support extensions to an exception callback would be to mark
the BPF_PROG_TYPE_EXT main subprog as an exception_cb, and prevent it
from calling bpf_throw. This would make the JIT produce a prologue that
restores saved registers and reset the stack frame. But let's not do
that until there is a concrete use case for this, and simply disallow
this for now.

One key point here to note is that currently X86_TAIL_CALL_OFFSET didn't
require any modifications, even though we emit instructions before the
corresponding endbr64 instruction. This is because we ensure that a main
subprog never serves as an exception callback, and therefore the
exception callback (which will be a global subprog) can never serve as
the tail call target, eliminating any discrepancies. However, once we
support a BPF_PROG_TYPE_EXT to also act as an exception callback, it
will end up requiring change to the tail call offset to account for the
extra instructions. For simplicitly, tail calls could be disabled for
such targets.

Noting the above, it appears better to wait for a concrete use case
before choosing to permit extension programs to replace exception
callbacks.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/helpers.c  | 1 +
 kernel/bpf/verifier.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 64a07232c58f..a04eff53354c 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2470,6 +2470,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
 	 */
 	kasan_unpoison_task_stack_below((void *)ctx.sp);
 	ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp);
+	WARN(1, "A call to BPF exception callback should never return\n");
 }
 
 __diag_pop();
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a0e1a1d1f5d3..13db1fa4163c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19622,6 +19622,11 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 					"Extension programs should be JITed\n");
 				return -EINVAL;
 			}
+			if (aux->func && aux->func[subprog]->aux->exception_cb) {
+				bpf_log(log,
+					"Extension programs cannot replace exception callback\n");
+				return -EINVAL;
+			}
 		}
 		if (!tgt_prog->jited) {
 			bpf_log(log, "Can attach to only JITed progs\n");
-- 
2.41.0


  parent reply	other threads:[~2023-08-09 11:43 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-09 11:41 [PATCH bpf-next v2 00/14] Exceptions - 1/2 Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 01/14] arch/x86: Implement arch_bpf_stack_walk Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 02/14] bpf: Implement support for adding hidden subprogs Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 03/14] bpf: Implement BPF exceptions Kumar Kartikeya Dwivedi
2023-08-22  5:12   ` Alexei Starovoitov
2023-08-22 12:53     ` Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 04/14] bpf: Refactor check_btf_func and split into two phases Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 05/14] bpf: Add support for custom exception callbacks Kumar Kartikeya Dwivedi
2023-08-28 22:11   ` Martin KaFai Lau
2023-08-09 11:41 ` [PATCH bpf-next v2 06/14] bpf: Perform CFG walk for exception callback Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 07/14] bpf: Treat first argument as return value for bpf_throw Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 08/14] bpf: Prevent KASAN false positive with bpf_throw Kumar Kartikeya Dwivedi
2023-08-22 16:23   ` Alexei Starovoitov
2023-08-30 16:53   ` Andrey Konovalov
2023-08-09 11:41 ` [PATCH bpf-next v2 09/14] bpf: Detect IP == ksym.end as part of BPF program Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` Kumar Kartikeya Dwivedi [this message]
2023-08-22  5:09   ` [PATCH bpf-next v2 10/14] bpf: Disallow extensions to exception callbacks Alexei Starovoitov
2023-08-22 12:53     ` Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 11/14] bpf: Fix kfunc callback register type handling Kumar Kartikeya Dwivedi
2023-08-10 21:12   ` David Marchevsky
2023-08-09 11:41 ` [PATCH bpf-next v2 12/14] libbpf: Add support for custom exception callbacks Kumar Kartikeya Dwivedi
2023-08-22 16:34   ` Alexei Starovoitov
2023-08-22 16:58     ` Kumar Kartikeya Dwivedi
2023-08-22 19:20       ` Alexei Starovoitov
2023-08-25 18:43   ` Andrii Nakryiko
2023-08-26 22:41     ` Kumar Kartikeya Dwivedi
2023-08-27 22:27       ` Alexei Starovoitov
2023-08-09 11:41 ` [PATCH bpf-next v2 13/14] selftests/bpf: Add BPF assertion macros Kumar Kartikeya Dwivedi
2023-08-09 11:41 ` [PATCH bpf-next v2 14/14] selftests/bpf: Add tests for BPF exceptions Kumar Kartikeya Dwivedi
2023-08-22 21:22 ` [PATCH bpf-next v2 00/14] Exceptions - 1/2 Kumar Kartikeya Dwivedi
2023-08-22 22:07   ` Jose E. Marchesi
2023-08-22 22:39     ` Yonghong Song
2023-08-22 22:53       ` Kumar Kartikeya Dwivedi
2023-08-22 23:06         ` Alexei Starovoitov
2023-08-25 18:55           ` Andrii Nakryiko
2023-08-26 22:42             ` Kumar Kartikeya Dwivedi
2023-08-22 22:54     ` Kumar Kartikeya Dwivedi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230809114116.3216687-11-memxor@gmail.com \
    --to=memxor@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=martin.lau@linux.dev \
    --cc=void@manifault.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.