Re: [PATCH v5 08/10] hornet: Add a light skeleton data extractor scripts

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Payload data corruption due to grep matching multiple lines and printf format string reuse.
- [Medium] Scripts fail to preprocess headers because gcc is invoked without include paths.
- [Medium] Pipeline failure when processing BPF programs with empty map data.
- [Low] Copy-paste error in usage text and unsafe unquoted variable usage.
--

commit 5d4574520f6f666709ea15ecc132a023d9222886
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

hornet: Add a light skeleton data extractor scripts

This commit introduces bash scripts to extract instructions and map data from
autogenerated eBPF light skeleton headers. These payloads can be used for
signing light skeleton binaries.

> diff --git a/scripts/hornet/extract-insn.sh b/scripts/hornet/extract-insn.sh
> new file mode 100755
> index 0000000000000..52338f057ff6b
> --- /dev/null
> +++ b/scripts/hornet/extract-insn.sh
[ ... ]
> +if [ $ARGC -ne $EXPECTED_ARGS ] ; then
> +    usage
> +else
> +    printf $(gcc -E $1 | grep "opts_insn" | \
> +		 awk -F"=" '{print $2}' | sed 's/;\+$//' | sed 's/\"//g')
> +fi

Will this corrupt the binary payload if grep matches multiple lines?

Skeleton headers generated by bpftool typically contain additional
references like opts.insns_sz = sizeof(opts_insn) - 1; and
opts.insns = (void *)opts_insn;.

Because the $(...) command substitution is unquoted, Bash will split the
output into multiple arguments. Since printf doesn't receive format
specifiers, it will reuse the binary payload as the format string for each
extra argument (such as sizeof, -, 1), repeating the payload multiple times
in the output.

Also, because EXPECTED_ARGS=1 is strictly enforced, users cannot pass -I
flags. Since the generated headers unconditionally include <bpf/bpf.h> and
<bpf/skel_internal.h>, will gcc -E fail and terminate before emitting the
target variables if those headers are not in the default system include path?

Could $1 be quoted as "$1" to prevent failures when the file path
contains spaces?

> diff --git a/scripts/hornet/extract-map.sh b/scripts/hornet/extract-map.sh
> new file mode 100755
> index 0000000000000..c309f505c6238
> --- /dev/null
> +++ b/scripts/hornet/extract-map.sh
[ ... ]
> +function usage() {
> +    echo "Sample script for extracting instructions"

This isn't a bug, but should this say "extracting map data" instead of
"instructions" since this is the map extractor script?

> +    echo "autogenerated eBPF lskel headers"
> +    echo ""
> +    echo "USAGE: header_file"
> +    exit
> +}
[ ... ]

> diff --git a/scripts/hornet/extract-skel.sh b/scripts/hornet/extract-skel.sh
> new file mode 100755
> index 0000000000000..6550a86b89917
> --- /dev/null
> +++ b/scripts/hornet/extract-skel.sh
[ ... ]
> +if [ $ARGC -ne $EXPECTED_ARGS ] ; then
> +    usage
> +else
> +    printf $(gcc -E $1 | grep "static const char opts_$2" | \
> +		 awk -F"=" '{print $2}' | sed 's/;\+$//' | sed 's/\"//g')
> +fi

If a BPF program uses no maps, the extracted skeleton will declare
static const char opts_data[] = "";. The awk and sed pipeline will strip
the quotes, resulting in an empty command substitution.

Since calling printf without arguments triggers a shell usage error
(bash: printf: usage: printf [-v var] format [arguments]) and returns a
non-zero exit status, will this break automated pipelines for map-less
BPF programs?

Additionally, since extract-skel.sh avoids the multiple-match issue by using
a stricter grep pattern (grep "static const char opts_$2"), does this make
extract-insn.sh and extract-map.sh redundant?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420212653.438685-1-bboscaccy@linux.microsoft.com?part=8