Re: [PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments

On Mon, Aug 10, 2026 at 7:23 AM <[email protected]> wrote:
>
> From: Yuan Chen <[email protected]>
>
> 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 <[email protected]>
> ---
>  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
>