BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues
@ 2026-08-10 14:22 chenyuan_fl
  2026-08-10 14:22 ` [PATCH bpf-next v5 1/3] bpftool: fix double close in map dump chenyuan_fl
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: chenyuan_fl @ 2026-08-10 14:22 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

This series fixes three issues in bpftool's batch file handling:

 1. map dump closes the map fd twice (introduced by the map-by-name
    dump refactor); do_show_subset() has the same double close.
 2. do_batch() reports a spurious read failure because a stale errno
    from a previously executed command is used instead of checking the
    FILE stream state.
 3. a line longer than the batch buffer that contains a '#' within the
    buffered prefix bypasses the line length check: the comment
    stripping truncates the buffer before the length is checked, so the
    unread remainder of the line is parsed and executed as a separate
    command on the next loop iteration. Continuation lines are affected
    the same way, bypassing the "command is too long" check.

Changes in v5:
  - Patch 1 also fixes the double close in do_show_subset() and
    propagates the error when bpf_map_get_info_by_fd() fails on a
    subsequent map in do_dump(), as pointed out in review.
  - Patch 3 detects the truncated read with memchr() for a newline plus
    feof() instead of strlen(), which is also fooled by an embedded NUL
    byte; a line that fills the buffer exactly is no longer rejected,
    as pointed out in review.

Changes in v4:
  - Extend patch 3 to also cover continuation lines, whose comment
    stripping bypasses the "command is too long" check the same way, as
    pointed out in review.
  - Drop the USE_LIBCAP errno reset in main(), which existed only to
    keep errno clean for the batch mode and is no longer needed now
    that do_batch() detects read failures with ferror(), as pointed out
    in review.

Changes in v3:
  - Add patch 3 fixing the pre-existing comment-stripping bypass of the
    batch line length check, as pointed out in review.

Yuan Chen (3):
  bpftool: fix double close in map dump
  bpftool: fix spurious batch file read error
  bpftool: Fix bypass of the batch line length check by comments

 tools/bpf/bpftool/main.c | 44 +++++++++++++++++++++++++++-------------
 tools/bpf/bpftool/map.c  |  4 +---
 2 files changed, 31 insertions(+), 17 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v5 1/3] bpftool: fix double close in map dump
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
@ 2026-08-10 14:22 ` chenyuan_fl
  2026-08-10 14:22 ` [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error chenyuan_fl
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: chenyuan_fl @ 2026-08-10 14:22 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

map_dump() closes the map fd in its error path, and do_dump() then
closes the same fd again after a successful dump. Closing an already
closed fd leaves errno set to EBADF, which poisons later errno checks
such as the batch file read check in do_batch(). Let do_dump() own the
fd and remove the close from map_dump().

The same double-close pattern exists in do_show_subset(): both
show_map_close_json() and show_map_close_plain() already close the fd,
so drop the extra close() there as well.

Also propagate the error when bpf_map_get_info_by_fd() fails on a
subsequent map in do_dump(): set err = -1 before breaking out of the
loop, so a later failure is not silently hidden after an earlier
iteration succeeded.

Fixes: 99f9863a0c45f ("bpftool: Match maps by name")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 tools/bpf/bpftool/map.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 6b9649294ca1..684a8fb72414 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -659,8 +659,6 @@ static int do_show_subset(int argc, char **argv)
 			show_map_close_json(fds[i], &info);
 		else
 			show_map_close_plain(fds[i], &info);
-
-		close(fds[i]);
 	}
 	if (json_output && nb_fds > 1)
 		jsonw_end_array(json_wtr);	/* root array */
@@ -895,7 +893,6 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
 exit_free:
 	free(key);
 	free(value);
-	close(fd);
 	free_map_kv_btf(btf);
 
 	return err;
@@ -944,6 +941,7 @@ static int do_dump(int argc, char **argv)
 	for (i = 0; i < nb_fds; i++) {
 		if (bpf_map_get_info_by_fd(fds[i], &info, &len)) {
 			p_err("can't get map info: %s", strerror(errno));
+			err = -1;
 			break;
 		}
 		err = map_dump(fds[i], &info, wtr, nb_fds > 1);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
  2026-08-10 14:22 ` [PATCH bpf-next v5 1/3] bpftool: fix double close in map dump chenyuan_fl
@ 2026-08-10 14:22 ` chenyuan_fl
  2026-08-13 21:56   ` Andrii Nakryiko
  2026-08-10 14:22 ` [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments chenyuan_fl
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: chenyuan_fl @ 2026-08-10 14:22 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

do_batch() checks errno after the read loop to detect read failures,
but fgets() does not clear errno on success, so a stale errno left by
a previously executed command (e.g. map dump's EBADF from a double
close) makes bpftool report a batch file read failure and exit with an
error even though every command succeeded. Use ferror() instead, and
track the too-long-line case explicitly.

Since do_batch() no longer inspects errno to detect read failures, drop
the USE_LIBCAP errno reset in main() that existed only to keep errno
clean for the batch mode.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 tools/bpf/bpftool/main.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index c91e1a6e1a1e..0bbb2e198450 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -336,6 +336,7 @@ static int do_batch(int argc, char **argv)
 	char buf[BATCH_LINE_LEN_MAX], contline[BATCH_LINE_LEN_MAX];
 	char *n_argv[BATCH_ARG_NB_MAX];
 	unsigned int lines = 0;
+	bool line_too_long = false;
 	int n_argc;
 	FILE *fp;
 	char *cp;
@@ -371,7 +372,7 @@ static int do_batch(int argc, char **argv)
 			*cp = '\0';
 
 		if (strlen(buf) == sizeof(buf) - 1) {
-			errno = E2BIG;
+			line_too_long = true;
 			break;
 		}
 
@@ -429,7 +430,10 @@ static int do_batch(int argc, char **argv)
 		lines++;
 	}
 
-	if (errno && errno != ENOENT) {
+	if (line_too_long) {
+		p_err("reading batch file failed: %s", strerror(E2BIG));
+		err = -1;
+	} else if (ferror(fp)) {
 		p_err("reading batch file failed: %s", strerror(errno));
 		err = -1;
 	} else {
@@ -467,16 +471,6 @@ int main(int argc, char **argv)
 
 	setlinebuf(stdout);
 
-#ifdef USE_LIBCAP
-	/* Libcap < 2.63 hooks before main() to compute the number of
-	 * capabilities of the running kernel, and doing so it calls prctl()
-	 * which may fail and set errno to non-zero.
-	 * Let's reset errno to make sure this does not interfere with the
-	 * batch mode.
-	 */
-	errno = 0;
-#endif
-
 	last_do_help = do_help;
 	pretty_output = false;
 	json_output = false;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
  2026-08-10 14:22 ` [PATCH bpf-next v5 1/3] bpftool: fix double close in map dump chenyuan_fl
  2026-08-10 14:22 ` [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error chenyuan_fl
@ 2026-08-10 14:22 ` chenyuan_fl
  2026-08-10 14:44   ` sashiko-bot
  2026-08-13 22:00   ` Andrii Nakryiko
  2026-08-24  9:26 ` [PATCH bpf-next v6 0/2] bpftool: fix batch file handling issues Yuan Chen
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: chenyuan_fl @ 2026-08-10 14:22 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

From: Yuan Chen <chenyuan@kylinos.cn>

do_batch() strips trailing comments by truncating the line at '#'
before checking whether fgets() filled the buffer. If a batch line
longer than the buffer contains a '#' within the first
sizeof(buf) - 1 bytes, the truncation makes strlen(buf) smaller and the
line-length check is bypassed. The unread remainder of the line then
stays in the file stream and is parsed and executed as a separate
command on the next loop iteration.

Continuation lines handled below are affected the same way: an overlong
continuation line containing '#' bypasses the "command is too
long" check, and its unread remainder is executed as a separate command.

Detect the truncated read before stripping the comment, using memchr()
to look for a newline (instead of strlen(), which is also fooled by an
embedded NUL byte) and feof() to tell an overlong line apart from a
final line without a trailing newline. A line that fills the buffer
exactly (the byte after the read is a newline) is not treated as
truncated, so valid maximal-length lines are no longer rejected. Use the
result for the line-length checks, so overlong lines are rejected
regardless of comments or NUL bytes.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 tools/bpf/bpftool/main.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 0bbb2e198450..337fbf1ec596 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -367,11 +367,27 @@ static int do_batch(int argc, char **argv)
 	if (json_output)
 		jsonw_start_array(json_wtr);
 	while (fgets(buf, sizeof(buf), fp)) {
+		bool truncated = !memchr(buf, '\n', sizeof(buf) - 1) && !feof(fp);
+
+		if (truncated) {
+			/*
+			 * fgets() filled the buffer. If the very next byte is
+			 * a newline, the line fits the buffer exactly and is
+			 * not truncated.
+			 */
+			int c = fgetc(fp);
+
+			if (c == '\n')
+				truncated = false;
+			else if (c != EOF)
+				ungetc(c, fp);
+		}
+
 		cp = strchr(buf, '#');
 		if (cp)
 			*cp = '\0';
 
-		if (strlen(buf) == sizeof(buf) - 1) {
+		if (truncated) {
 			line_too_long = true;
 			break;
 		}
@@ -380,6 +396,8 @@ static int do_batch(int argc, char **argv)
 		 * with '\' in the batch file).
 		 */
 		while ((cp = strstr(buf, "\\\n")) != NULL) {
+			bool cont_truncated;
+
 			if (!fgets(contline, sizeof(contline), fp) ||
 			    strlen(contline) == 0) {
 				p_err("missing continuation line on command %u",
@@ -388,11 +406,15 @@ static int do_batch(int argc, char **argv)
 				goto err_close;
 			}
 
+			cont_truncated = !memchr(contline, '\n', sizeof(contline) - 1) &&
+					 !feof(fp);
+
 			cp = strchr(contline, '#');
 			if (cp)
 				*cp = '\0';
 
-			if (strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
+			if (cont_truncated ||
+			    strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
 				p_err("command %u is too long", lines);
 				err = -1;
 				goto err_close;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments
  2026-08-10 14:22 ` [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments chenyuan_fl
@ 2026-08-10 14:44   ` sashiko-bot
  2026-08-13 22:00   ` Andrii Nakryiko
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-10 14:44 UTC (permalink / raw)
  To: chenyuan_fl; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Uninitialized memory read in `memchr` when `fgets` reads a short line without a newline.
- [Medium] Discarded newline character breaks the command continuation logic for maximal-length lines.
--

--- Patch [3]: [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments ---
commit 6f345d4b0d20421906e194cdb508c0d3bd38e98a
Author: Yuan Chen <chenyuan@kylinos.cn>

bpftool: Fix bypass of the batch line length check by comments

This commit fixes a bug in bpftool where batch lines containing a '#'
could bypass the line length check, leading to the remainder of the
line being executed as a separate command. It introduces memchr() and
feof() checks to accurately detect truncated reads.

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 0bbb2e1984503..337fbf1ec596a 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -367,11 +367,27 @@ static int do_batch(int argc, char **argv)
>  	if (json_output)
>  		jsonw_start_array(json_wtr);
>  	while (fgets(buf, sizeof(buf), fp)) {
> +		bool truncated = !memchr(buf, '\n', sizeof(buf) - 1) && !feof(fp);

[Severity: Medium]
If a short command that does not end in a newline is read, fgets() will
append a null terminator and leave the rest of the stack buffer uninitialized.

Does this memchr() call result in an uninitialized memory read by
unconditionally scanning the full buffer size past the null terminator?

> +
> +		if (truncated) {
> +			/*
> +			 * fgets() filled the buffer. If the very next byte is
> +			 * a newline, the line fits the buffer exactly and is
> +			 * not truncated.
> +			 */
> +			int c = fgetc(fp);
> +
> +			if (c == '\n')
> +				truncated = false;

[Severity: Medium]
When a command line reaches exactly the maximum buffer size minus one and ends
with a continuation backslash, fgets() fills the buffer and stops before
reading the newline.

If we read the newline character here and discard it without putting it back
in the stream or adding it to the buffer, will it break the continuation logic?

Later in do_batch(), the code checks for the presence of the newline alongside
the backslash:

    while ((cp = strstr(buf, "\\\n")) != NULL) {

Will this strstr() check fail since the newline was permanently consumed?

> +			else if (c != EOF)
> +				ungetc(c, fp);
> +		}

[ ... ]

> @@ -380,6 +396,8 @@ static int do_batch(int argc, char **argv)
>  		 * with '\' in the batch file).
>  		 */
>  		while ((cp = strstr(buf, "\\\n")) != NULL) {
> +			bool cont_truncated;
> +
>  			if (!fgets(contline, sizeof(contline), fp) ||
>  			    strlen(contline) == 0) {
>  				p_err("missing continuation line on command %u",
> @@ -388,11 +406,15 @@ static int do_batch(int argc, char **argv)
>  				goto err_close;
>  			}
>  
> +			cont_truncated = !memchr(contline, '\n', sizeof(contline) - 1) &&
> +					 !feof(fp);

[Severity: Medium]
Does this memchr() call introduce the same uninitialized memory read regression
for the continuation line buffer as the one above?

> +
>  			cp = strchr(contline, '#');
>  			if (cp)
>  				*cp = '\0';

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810142224.2907373-1-chenyuan_fl@163.com?part=3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error
  2026-08-10 14:22 ` [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error chenyuan_fl
@ 2026-08-13 21:56   ` Andrii Nakryiko
  0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2026-08-13 21:56 UTC (permalink / raw)
  To: chenyuan_fl
  Cc: bpf, Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

On Mon, Aug 10, 2026 at 7:23 AM <chenyuan_fl@163.com> wrote:
>
> From: Yuan Chen <chenyuan@kylinos.cn>
>
> do_batch() checks errno after the read loop to detect read failures,
> but fgets() does not clear errno on success, so a stale errno left by
> a previously executed command (e.g. map dump's EBADF from a double
> close) makes bpftool report a batch file read failure and exit with an
> error even though every command succeeded. Use ferror() instead, and
> track the too-long-line case explicitly.
>
> Since do_batch() no longer inspects errno to detect read failures, drop
> the USE_LIBCAP errno reset in main() that existed only to keep errno
> clean for the batch mode.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  tools/bpf/bpftool/main.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c91e1a6e1a1e..0bbb2e198450 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -336,6 +336,7 @@ static int do_batch(int argc, char **argv)
>         char buf[BATCH_LINE_LEN_MAX], contline[BATCH_LINE_LEN_MAX];
>         char *n_argv[BATCH_ARG_NB_MAX];
>         unsigned int lines = 0;
> +       bool line_too_long = false;

it feels wrong to track this specific error explicitly. we already
have err, so we can check that err is set and then look at errno. or
clear errno before fgets(), some variant of that maybe. but not
explicit bool like what you did here.

I've applied the first patch, but please iteration on this one

>         int n_argc;
>         FILE *fp;
>         char *cp;
> @@ -371,7 +372,7 @@ static int do_batch(int argc, char **argv)
>                         *cp = '\0';
>
>                 if (strlen(buf) == sizeof(buf) - 1) {
> -                       errno = E2BIG;
> +                       line_too_long = true;
>                         break;
>                 }
>
> @@ -429,7 +430,10 @@ static int do_batch(int argc, char **argv)
>                 lines++;
>         }
>
> -       if (errno && errno != ENOENT) {
> +       if (line_too_long) {
> +               p_err("reading batch file failed: %s", strerror(E2BIG));
> +               err = -1;
> +       } else if (ferror(fp)) {
>                 p_err("reading batch file failed: %s", strerror(errno));
>                 err = -1;
>         } else {
> @@ -467,16 +471,6 @@ int main(int argc, char **argv)
>
>         setlinebuf(stdout);
>
> -#ifdef USE_LIBCAP
> -       /* Libcap < 2.63 hooks before main() to compute the number of
> -        * capabilities of the running kernel, and doing so it calls prctl()
> -        * which may fail and set errno to non-zero.
> -        * Let's reset errno to make sure this does not interfere with the
> -        * batch mode.
> -        */
> -       errno = 0;
> -#endif
> -
>         last_do_help = do_help;
>         pretty_output = false;
>         json_output = false;
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments
  2026-08-10 14:22 ` [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments chenyuan_fl
  2026-08-10 14:44   ` sashiko-bot
@ 2026-08-13 22:00   ` Andrii Nakryiko
  1 sibling, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2026-08-13 22:00 UTC (permalink / raw)
  To: chenyuan_fl
  Cc: bpf, Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Yuan Chen

On Mon, Aug 10, 2026 at 7:23 AM <chenyuan_fl@163.com> wrote:
>
> From: Yuan Chen <chenyuan@kylinos.cn>
>
> do_batch() strips trailing comments by truncating the line at '#'
> before checking whether fgets() filled the buffer. If a batch line
> longer than the buffer contains a '#' within the first
> sizeof(buf) - 1 bytes, the truncation makes strlen(buf) smaller and the
> line-length check is bypassed. The unread remainder of the line then
> stays in the file stream and is parsed and executed as a separate
> command on the next loop iteration.
>
> Continuation lines handled below are affected the same way: an overlong
> continuation line containing '#' bypasses the "command is too
> long" check, and its unread remainder is executed as a separate command.
>
> Detect the truncated read before stripping the comment, using memchr()
> to look for a newline (instead of strlen(), which is also fooled by an
> embedded NUL byte) and feof() to tell an overlong line apart from a
> final line without a trailing newline. A line that fills the buffer
> exactly (the byte after the read is a newline) is not treated as
> truncated, so valid maximal-length lines are no longer rejected. Use the
> result for the line-length checks, so overlong lines are rejected
> regardless of comments or NUL bytes.
>
> Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---
>  tools/bpf/bpftool/main.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 0bbb2e198450..337fbf1ec596 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -367,11 +367,27 @@ static int do_batch(int argc, char **argv)
>         if (json_output)
>                 jsonw_start_array(json_wtr);
>         while (fgets(buf, sizeof(buf), fp)) {
> +               bool truncated = !memchr(buf, '\n', sizeof(buf) - 1) && !feof(fp);
> +
> +               if (truncated) {
> +                       /*
> +                        * fgets() filled the buffer. If the very next byte is
> +                        * a newline, the line fits the buffer exactly and is
> +                        * not truncated.
> +                        */
> +                       int c = fgetc(fp);
> +
> +                       if (c == '\n')
> +                               truncated = false;
> +                       else if (c != EOF)
> +                               ungetc(c, fp);
> +               }
> +
>                 cp = strchr(buf, '#');
>                 if (cp)
>                         *cp = '\0';
>
> -               if (strlen(buf) == sizeof(buf) - 1) {

just move this check before you overwrite buf contents. even with
comments, a reasonable line shouldn't be 64KB long, no?

pw-bot: cr

> +               if (truncated) {
>                         line_too_long = true;
>                         break;
>                 }
> @@ -380,6 +396,8 @@ static int do_batch(int argc, char **argv)
>                  * with '\' in the batch file).
>                  */
>                 while ((cp = strstr(buf, "\\\n")) != NULL) {
> +                       bool cont_truncated;
> +
>                         if (!fgets(contline, sizeof(contline), fp) ||
>                             strlen(contline) == 0) {
>                                 p_err("missing continuation line on command %u",
> @@ -388,11 +406,15 @@ static int do_batch(int argc, char **argv)
>                                 goto err_close;
>                         }
>
> +                       cont_truncated = !memchr(contline, '\n', sizeof(contline) - 1) &&
> +                                        !feof(fp);
> +
>                         cp = strchr(contline, '#');
>                         if (cp)
>                                 *cp = '\0';
>
> -                       if (strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
> +                       if (cont_truncated ||
> +                           strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
>                                 p_err("command %u is too long", lines);
>                                 err = -1;
>                                 goto err_close;
> --
> 2.54.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v6 0/2] bpftool: fix batch file handling issues
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
                   ` (2 preceding siblings ...)
  2026-08-10 14:22 ` [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments chenyuan_fl
@ 2026-08-24  9:26 ` Yuan Chen
  2026-08-24  9:26 ` [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error Yuan Chen
  2026-08-24  9:26 ` [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments Yuan Chen
  5 siblings, 0 replies; 13+ messages in thread
From: Yuan Chen @ 2026-08-24  9:26 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Andrii Nakryiko, Yuan Chen

This series fixes two remaining issues in bpftool's batch file handling
(the double-close fix from v5 has been applied upstream and is no
longer part of this series):

 1. do_batch() reports a spurious read failure because a stale errno
    left by a previously executed command is checked after the read
    loop instead of the outcome of the last fgets() call.
 2. a line longer than the batch buffer that contains a '#' within the
    buffered prefix bypasses the line length check: the comment
    stripping truncates the buffer before the length is checked, so the
    unread remainder of the line is parsed and executed as a separate
    command on the next loop iteration. Continuation lines are affected
    the same way, bypassing the "command is too long" check.

Many thanks to Andrii Nakryiko for the review: patch 1 now clears errno
before each fgets() call instead of tracking the too-long-line case
with an explicit flag, and patch 2 moves the line-length checks before
the comment stripping instead of detecting truncated reads with
memchr()/feof().

Changes in v6:
  - drop the double-close patch, which has been applied upstream; the
    series is now two patches
  - "fix spurious batch file read error": clear errno before each
    fgets() call, and drop the explicit line_too_long flag and the
    ferror() check (reviewer feedback)
  - "Fix bypass of the batch line length check by comments": move the
    line-length checks before the comment stripping, dropping the
    memchr()/feof() truncation detection (reviewer feedback)

Yuan Chen (2):
  bpftool: fix spurious batch file read error
  bpftool: Fix bypass of the batch line length check by comments

 tools/bpf/bpftool/main.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
                   ` (3 preceding siblings ...)
  2026-08-24  9:26 ` [PATCH bpf-next v6 0/2] bpftool: fix batch file handling issues Yuan Chen
@ 2026-08-24  9:26 ` Yuan Chen
  2026-08-24 10:02   ` bot+bpf-ci
  2026-08-28  0:10   ` patchwork-bot+netdevbpf
  2026-08-24  9:26 ` [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments Yuan Chen
  5 siblings, 2 replies; 13+ messages in thread
From: Yuan Chen @ 2026-08-24  9:26 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Andrii Nakryiko, Yuan Chen, Yuan Chen

do_batch() checks errno after the read loop to detect read failures,
but fgets() does not clear errno on success, so a stale errno left by
a previously executed command (e.g. map dump's EBADF from a double
close) makes bpftool report a batch file read failure and exit with an
error even though every command succeeded.

Clear errno before each fgets() call, so the post-loop check only
sees the outcome of the last read: zero on success or EOF, E2BIG for
an overlong line, and a genuine errno when fgets() fails.

Since errno is now reset before every read in batch mode, drop the
USE_LIBCAP errno reset in main() that existed only to keep errno clean
for the batch mode.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
v6: clear errno before each fgets() call instead of tracking the
    too-long-line case with an explicit flag and checking ferror(), as
    suggested by Andrii Nakryiko

 tools/bpf/bpftool/main.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index c91e1a6e1a1e..7a0c214f08a0 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -365,7 +365,11 @@
 
 	if (json_output)
 		jsonw_start_array(json_wtr);
-	while (fgets(buf, sizeof(buf), fp)) {
+	for (;;) {
+		errno = 0;
+		if (!fgets(buf, sizeof(buf), fp))
+			break;
+
 		cp = strchr(buf, '#');
 		if (cp)
 			*cp = '\0';
@@ -467,16 +471,6 @@
 
 	setlinebuf(stdout);
 
-#ifdef USE_LIBCAP
-	/* Libcap < 2.63 hooks before main() to compute the number of
-	 * capabilities of the running kernel, and doing so it calls prctl()
-	 * which may fail and set errno to non-zero.
-	 * Let's reset errno to make sure this does not interfere with the
-	 * batch mode.
-	 */
-	errno = 0;
-#endif
-
 	last_do_help = do_help;
 	pretty_output = false;
 	json_output = false;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments
  2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
                   ` (4 preceding siblings ...)
  2026-08-24  9:26 ` [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error Yuan Chen
@ 2026-08-24  9:26 ` Yuan Chen
  2026-08-28  0:10   ` patchwork-bot+netdevbpf
  5 siblings, 1 reply; 13+ messages in thread
From: Yuan Chen @ 2026-08-24  9:26 UTC (permalink / raw)
  To: bpf
  Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Andrii Nakryiko, Yuan Chen, Yuan Chen

do_batch() strips trailing comments by truncating the line at '#'
before checking whether fgets() filled the buffer. If a batch line
longer than the buffer contains a '#' within the first
sizeof(buf) - 1 bytes, the truncation makes strlen(buf) smaller and the
line-length check is bypassed. The unread remainder of the line then
stays in the file stream and is parsed and executed as a separate
command on the next loop iteration.

Continuation lines handled below are affected the same way: an overlong
continuation line containing '#' bypasses the "command is too long"
check, and its unread remainder is executed as a separate command.

Move the line-length checks before the comment is stripped, so they see
the full line as read from the file and overlong lines are rejected
regardless of comments. A line that fills the buffer exactly is now
rejected as well, which is fine: batch command lines are not expected
to come anywhere near the buffer limit.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
v6: move the line-length checks before comment stripping instead of
    detecting truncated reads with memchr()/feof(), as suggested by
    Andrii Nakryiko

 tools/bpf/bpftool/main.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 7a0c214f08a0..5ababd8f7d0a 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -370,15 +370,15 @@
 		if (!fgets(buf, sizeof(buf), fp))
 			break;
 
-		cp = strchr(buf, '#');
-		if (cp)
-			*cp = '\0';
-
 		if (strlen(buf) == sizeof(buf) - 1) {
 			errno = E2BIG;
 			break;
 		}
 
+		cp = strchr(buf, '#');
+		if (cp)
+			*cp = '\0';
+
 		/* Append continuation lines if any (coming after a line ending
 		 * with '\' in the batch file).
 		 */
@@ -391,15 +391,15 @@
 				goto err_close;
 			}
 
-			cp = strchr(contline, '#');
-			if (cp)
-				*cp = '\0';
-
 			if (strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
 				p_err("command %u is too long", lines);
 				err = -1;
 				goto err_close;
 			}
+
+			cp = strchr(contline, '#');
+			if (cp)
+				*cp = '\0';
 			buf[strlen(buf) - 2] = '\0';
 			strcat(buf, contline);
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error
  2026-08-24  9:26 ` [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error Yuan Chen
@ 2026-08-24 10:02   ` bot+bpf-ci
  2026-08-28  0:10   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 13+ messages in thread
From: bot+bpf-ci @ 2026-08-24 10:02 UTC (permalink / raw)
  To: chenyuan_fl, bpf
  Cc: qmo, ast, daniel, andrii, eddyz87, memxor, andrii.nakryiko,
	chenyuan_fl, chenyuan, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3068 bytes --]

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c91e1a6e1a1e..7a0c214f08a0 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -365,7 +365,11 @@ static int do_batch(int argc, char **argv)
>
>  	if (json_output)
>  		jsonw_start_array(json_wtr);
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	for (;;) {
> +		errno = 0;
> +		if (!fgets(buf, sizeof(buf), fp))
> +			break;
> +

[ ... ]

>  		if (errno && errno != ENOENT) {
>  			p_err("reading batch file failed: %s", strerror(errno));
>  			err = -1;

This isn't a bug, but now that errno is cleared before each fgets() and the
post-loop check only sees the last fgets() outcome, is the errno != ENOENT
exemption still needed?

Looking at the code, fgets() and read() cannot fail with ENOENT. The
exemption was load-bearing before this change: commands like map show
(map.c:740), link show (link.c:1356), and map dump (map.c:875) all return
with errno == ENOENT, and the exemption prevented those stale values from
triggering false failures. This patch fixes that at the source by clearing
errno before each read.

Would dropping the errno != ENOENT check here (similar to how the
USE_LIBCAP reset is removed) make the intent clearer?

>
> @@ -467,16 +471,6 @@ int main(int argc, char **argv)
>
>  	setlinebuf(stdout);
>
> -#ifdef USE_LIBCAP
> -	/* Libcap < 2.63 hooks before main() to compute the number of
> -	 * capabilities of the running kernel, and doing so it calls prctl()
> -	 * which may fail and set errno to non-zero.
> -	 * Let's reset errno to make sure this does not interfere with the
> -	 * batch mode.
> -	 */
> -	errno = 0;
> -#endif
> -
>  	last_do_help = do_help;
>  	pretty_output = false;
>  	json_output = false;

This isn't a bug, but could the changelog use an example that still
reproduces on current bpf-next?

The commit message cites "map dump's EBADF from a double close" as the
example of a stale errno:

>     do_batch() checks errno after the read loop to detect read failures,
>     but fgets() does not clear errno on success, so a stale errno left by
>     a previously executed command (e.g. map dump's EBADF from a double
>     close) makes bpftool report a batch file read failure and exit with an
>     error even though every command succeeded.

Looking at the current code, map_dump() no longer closes the fd
(tools/bpf/bpftool/map.c:893-898 just has exit_free: free(key);
free(value); free_map_kv_btf(btf);), and do_dump() is the sole owner
(map.c:941-962). So there's no double close and no EBADF left behind.

A successful map dump actually leaves errno == ENOENT from
bpf_map_get_next_key() exhaustion (map.c:874-877), which the existing
errno != ENOENT exemption already suppressed. This means map dump isn't
actually a command that could have triggered the symptom described.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32712601990

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error
  2026-08-24  9:26 ` [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error Yuan Chen
  2026-08-24 10:02   ` bot+bpf-ci
@ 2026-08-28  0:10   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-28  0:10 UTC (permalink / raw)
  To: Yuan Chen
  Cc: bpf, qmo, ast, daniel, andrii, eddyz87, memxor, andrii.nakryiko,
	chenyuan

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 24 Aug 2026 17:26:56 +0800 you wrote:
> do_batch() checks errno after the read loop to detect read failures,
> but fgets() does not clear errno on success, so a stale errno left by
> a previously executed command (e.g. map dump's EBADF from a double
> close) makes bpftool report a batch file read failure and exit with an
> error even though every command succeeded.
> 
> Clear errno before each fgets() call, so the post-loop check only
> sees the outcome of the last read: zero on success or EOF, E2BIG for
> an overlong line, and a genuine errno when fgets() fails.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v6,1/2] bpftool: fix spurious batch file read error
    https://git.kernel.org/bpf/bpf-next/c/713c02b59e21

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments
  2026-08-24  9:26 ` [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments Yuan Chen
@ 2026-08-28  0:10   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-28  0:10 UTC (permalink / raw)
  To: Yuan Chen
  Cc: bpf, qmo, ast, daniel, andrii, eddyz87, memxor, andrii.nakryiko,
	chenyuan

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 24 Aug 2026 17:26:57 +0800 you wrote:
> do_batch() strips trailing comments by truncating the line at '#'
> before checking whether fgets() filled the buffer. If a batch line
> longer than the buffer contains a '#' within the first
> sizeof(buf) - 1 bytes, the truncation makes strlen(buf) smaller and the
> line-length check is bypassed. The unread remainder of the line then
> stays in the file stream and is parsed and executed as a separate
> command on the next loop iteration.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v6,2/2] bpftool: Fix bypass of the batch line length check by comments
    https://git.kernel.org/bpf/bpf-next/c/ac3d88577cda

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-08-28  0:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 14:22 [PATCH bpf-next v5 0/3] bpftool: fix batch file handling issues chenyuan_fl
2026-08-10 14:22 ` [PATCH bpf-next v5 1/3] bpftool: fix double close in map dump chenyuan_fl
2026-08-10 14:22 ` [PATCH bpf-next v5 2/3] bpftool: fix spurious batch file read error chenyuan_fl
2026-08-13 21:56   ` Andrii Nakryiko
2026-08-10 14:22 ` [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments chenyuan_fl
2026-08-10 14:44   ` sashiko-bot
2026-08-13 22:00   ` Andrii Nakryiko
2026-08-24  9:26 ` [PATCH bpf-next v6 0/2] bpftool: fix batch file handling issues Yuan Chen
2026-08-24  9:26 ` [PATCH bpf-next v6 1/2] bpftool: fix spurious batch file read error Yuan Chen
2026-08-24 10:02   ` bot+bpf-ci
2026-08-28  0:10   ` patchwork-bot+netdevbpf
2026-08-24  9:26 ` [PATCH bpf-next v6 2/2] bpftool: Fix bypass of the batch line length check by comments Yuan Chen
2026-08-28  0:10   ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox