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

>     Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
>     Signed-off-by: Yuan Chen <[email protected]>

Does the Fixes: tag point to the right commit?

The bug being fixed is the ordering of the comment strip relative to the
line-length check. In commit 71bb428fe2c1 ("tools: bpf: add bpftool")
do_batch() had no '#' handling at all; the loop body started with the
length check immediately after fgets():

    while (fgets(buf, sizeof(buf), fp)) {
            if (strlen(buf) == sizeof(buf) - 1) {
                    errno = E2BIG;
                    break;
            }

So an overlong line was correctly rejected with E2BIG and the bypass
described in the changelog was not possible.

The defect was introduced by commit 06cc7fe7c295 ("tools: bpftool: support
comments in batch files"), which inserted the truncation directly above
that check:

    while (fgets(buf, sizeof(buf), fp)) {
    +       cp = strchr(buf, '#');
    +       if (cp)
    +               *cp = '\0';
    +
            if (strlen(buf) == sizeof(buf) - 1) {

That is the commit that made strlen(buf) shrink before the length test,
which is exactly what this patch reorders. The tag should read:

    Fixes: 06cc7fe7c295 ("tools: bpftool: support comments in batch files")

This also matters for stable backport scope: 71bb428fe2c1 landed in v4.15
while 06cc7fe7c295 landed in v4.16, so the current tag widens the backport
range to a kernel where the bug is absent.

Note the parent commit fbbb15fafea3 ("bpftool: fix spurious batch file read
error") does correctly carry Fixes: 71bb428fe2c1, since the errno-based
check it replaces was present in that commit; the tag appears to have been
carried over unchanged into this patch.

The code change itself is correct: recording 'truncated' from fgets()
before the '#' strip makes the E2BIG rejection independent of comments,
and the new local is block-scoped inside the loop body so it is
re-initialized on every iteration.

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c9a86039c400..c77e66bffbc9 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -367,11 +367,13 @@ static int do_batch(int argc, char **argv)
>  	if (json_output)
>  		jsonw_start_array(json_wtr);
>  	while (fgets(buf, sizeof(buf), fp)) {
> +		bool truncated = strlen(buf) == sizeof(buf) - 1;
> +
>  		cp = strchr(buf, '#');
>  		if (cp)
>  			*cp = '\0';
>
> -		if (strlen(buf) == sizeof(buf) - 1) {
> +		if (truncated) {
>  			line_too_long = true;
>  			break;
>  		}


---
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/31367518511