All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>,
	<martin.lau@kernel.org>, <lmb@isovalent.com>, <timo@incline.eu>,
	<robin.goegge@isovalent.com>
Cc: <andrii@kernel.org>, <kernel-team@meta.com>
Subject: [PATCH v4 bpf-next 14/19] bpf: relax log_buf NULL conditions when log_level>0 is requested
Date: Thu, 6 Apr 2023 16:42:00 -0700	[thread overview]
Message-ID: <20230406234205.323208-15-andrii@kernel.org> (raw)
In-Reply-To: <20230406234205.323208-1-andrii@kernel.org>

Drop the log_size>0 and log_buf!=NULL condition when log_level>0. This
allows users to request log_true_size of a full log without providing
actual (even if small) log buffer. Verifier log handling code was mostly
ready to handle NULL log->ubuf, so only few small changes were necessary
to prevent NULL log->ubuf from causing problems.

Note, that if user provided NULL log_buf with log_level>0 we don't
consider this a log truncation, and thus won't return -ENOSPC.

We also enforce that either (log_buf==NULL && log_size==0) or
(log_buf!=NULL && log_size>0).

Suggested-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/log.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index 1fae2c5d7ae4..046ddff37a76 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -12,8 +12,17 @@
 
 static bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log)
 {
-	return log->len_total > 0 && log->len_total <= UINT_MAX >> 2 &&
-	       log->level && log->ubuf && !(log->level & ~BPF_LOG_MASK);
+	/* ubuf and len_total should both be specified (or not) together */
+	if (!!log->ubuf != !!log->len_total)
+		return false;
+	/* log buf without log_level is meaningless */
+	if (log->ubuf && log->level == 0)
+		return false;
+	if (log->level & ~BPF_LOG_MASK)
+		return false;
+	if (log->len_total > UINT_MAX >> 2)
+		return false;
+	return true;
 }
 
 int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
@@ -89,9 +98,15 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
 			new_start = new_end - log->len_total;
 		else
 			new_start = log->start_pos;
+
+		log->start_pos = new_start;
+		log->end_pos = new_end - 1; /* don't count terminating '\0' */
+
+		if (!log->ubuf)
+			return;
+
 		new_n = min(n, log->len_total);
 		cur_pos = new_end - new_n;
-
 		div_u64_rem(cur_pos, log->len_total, &buf_start);
 		div_u64_rem(new_end, log->len_total, &buf_end);
 		/* new_end and buf_end are exclusive indices, so if buf_end is
@@ -101,12 +116,6 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt,
 		if (buf_end == 0)
 			buf_end = log->len_total;
 
-		log->start_pos = new_start;
-		log->end_pos = new_end - 1; /* don't count terminating '\0' */
-
-		if (!log->ubuf)
-			return;
-
 		/* if buf_start > buf_end, we wrapped around;
 		 * if buf_start == buf_end, then we fill ubuf completely; we
 		 * can't have buf_start == buf_end to mean that there is
@@ -156,12 +165,15 @@ void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos)
 	if (log->end_pos < log->start_pos)
 		log->start_pos = log->end_pos;
 
+	if (!log->ubuf)
+		return;
+
 	if (log->level & BPF_LOG_FIXED)
 		pos = log->end_pos + 1;
 	else
 		div_u64_rem(new_pos, log->len_total, &pos);
 
-	if (log->ubuf && pos < log->len_total && put_user(zero, log->ubuf + pos))
+	if (pos < log->len_total && put_user(zero, log->ubuf + pos))
 		log->ubuf = NULL;
 }
 
@@ -211,11 +223,6 @@ static int bpf_vlog_reverse_ubuf(struct bpf_verifier_log *log, int start, int en
 	return 0;
 }
 
-static bool bpf_vlog_truncated(const struct bpf_verifier_log *log)
-{
-	return log->len_max > log->len_total;
-}
-
 int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual)
 {
 	u32 sublen;
@@ -228,7 +235,7 @@ int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual)
 	if (!log->ubuf)
 		goto skip_log_rotate;
 	/* If we never truncated log, there is nothing to move around. */
-	if ((log->level & BPF_LOG_FIXED) || log->start_pos == 0)
+	if (log->start_pos == 0)
 		goto skip_log_rotate;
 
 	/* Otherwise we need to rotate log contents to make it start from the
@@ -283,7 +290,8 @@ int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual)
 	if (!!log->ubuf != !!log->len_total)
 		return -EFAULT;
 
-	if (bpf_vlog_truncated(log))
+	/* did truncation actually happen? */
+	if (log->ubuf && log->len_max > log->len_total)
 		return -ENOSPC;
 
 	return 0;
-- 
2.34.1


  parent reply	other threads:[~2023-04-06 23:43 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-06 23:41 [PATCH v4 bpf-next 00/19] BPF verifier rotating log Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 01/19] bpf: split off basic BPF verifier log into separate file Andrii Nakryiko
2023-04-11 10:43   ` Lorenz Bauer
2023-04-06 23:41 ` [PATCH v4 bpf-next 02/19] bpf: remove minimum size restrictions on verifier log buffer Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 03/19] bpf: switch BPF verifier log to be a rotating log by default Andrii Nakryiko
2023-04-11 10:28   ` Lorenz Bauer
2023-04-06 23:41 ` [PATCH v4 bpf-next 04/19] libbpf: don't enforce unnecessary verifier log restrictions on libbpf side Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-06 23:41 ` [PATCH v4 bpf-next 05/19] veristat: add more veristat control over verifier log options Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 06/19] selftests/bpf: add fixed vs rotating verifier log tests Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 07/19] bpf: ignore verifier log reset in BPF_LOG_KERNEL mode Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 08/19] bpf: fix missing -EFAULT return on user log buf error in btf_parse() Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 09/19] bpf: avoid incorrect -EFAULT error in BPF_LOG_KERNEL mode Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 10/19] bpf: simplify logging-related error conditions handling Andrii Nakryiko
2023-04-06 23:41 ` [PATCH v4 bpf-next 11/19] bpf: keep track of total log content size in both fixed and rolling modes Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-06 23:41 ` [PATCH v4 bpf-next 12/19] bpf: add log_true_size output field to return necessary log buffer size Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-06 23:41 ` [PATCH v4 bpf-next 13/19] bpf: simplify internal verifier log interface Andrii Nakryiko
2023-04-06 23:42 ` Andrii Nakryiko [this message]
2023-04-11 10:44   ` [PATCH v4 bpf-next 14/19] bpf: relax log_buf NULL conditions when log_level>0 is requested Lorenz Bauer
2023-04-06 23:42 ` [PATCH v4 bpf-next 15/19] libbpf: wire through log_true_size returned from kernel for BPF_PROG_LOAD Andrii Nakryiko
2023-04-06 23:42 ` [PATCH v4 bpf-next 16/19] libbpf: wire through log_true_size for bpf_btf_load() API Andrii Nakryiko
2023-04-06 23:42 ` [PATCH v4 bpf-next 17/19] selftests/bpf: add tests to validate log_true_size feature Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-06 23:42 ` [PATCH v4 bpf-next 18/19] selftests/bpf: add testing of log_buf==NULL condition for BPF_PROG_LOAD Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-06 23:42 ` [PATCH v4 bpf-next 19/19] selftests/bpf: add verifier log tests for BPF_BTF_LOAD command Andrii Nakryiko
2023-04-11 10:44   ` Lorenz Bauer
2023-04-11 14:03 ` [PATCH v4 bpf-next 00/19] BPF verifier rotating log Lorenz Bauer
2023-04-11 16:10 ` patchwork-bot+netdevbpf
2023-04-12 15:31 ` Lorenz Bauer
2023-04-12 17:03   ` Andrii Nakryiko

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=20230406234205.323208-15-andrii@kernel.org \
    --to=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=lmb@isovalent.com \
    --cc=martin.lau@kernel.org \
    --cc=robin.goegge@isovalent.com \
    --cc=timo@incline.eu \
    /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.