Re: [PATCH bpf-next 2/2] bpftool: fix spurious batch file read error

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the comment stripping logic bypasses the line truncation check, allowing portions of excessively long commented lines to be silently executed as separate commands.
--

--- Patch [2]: [PATCH bpf-next 2/2] bpftool: fix spurious batch file read error ---
commit 858e1807e2fde336612fcf607106fe8e9010fc61
Author: Yuan Chen <[email protected]>

bpftool: fix spurious batch file read error

This commit addresses a bug where do_batch() incorrectly detects a read
failure due to a stale errno from a previously executed command. It replaces
the errno check with ferror(fp) and explicitly tracks cases where a line
exceeds the buffer size limit.

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index c91e1a6e1a1ec..c9a86039c400b 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
[ ... ]
> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but doesn't the comment stripping
logic right above this in do_batch() bypass the line truncation check?

If a batch file line exceeds the buffer limit (4095 characters) and contains
a '#' character in the first 4095 bytes, fgets() reads the first 4095 bytes,
but the '#' causes the string to be truncated early by setting '*cp = '\0''. 
This means strlen(buf) becomes less than sizeof(buf) - 1, incorrectly bypassing
this check.

Could this allow the unread remainder of an excessively long commented line to
remain in the file stream, where it would be parsed and executed as a separate
command on the next loop iteration?

>  			break;
>  		}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2