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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Silent data corruption of the extracted eBPF payload due to unquoted printf format string injection.
- [Critical] Overly broad grep patterns in `extract-insn.sh` and `extract-map.sh` cause payload duplication and severe corruption.
- [High] Execution failure due to missing C preprocessor include paths.
- [Medium] Bash scripting pitfalls: Incorrect exit code on failure, command injection, and copy-paste error in usage.
--

commit 398e95d4981b28182554e7639f02b8c22e6185c8
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

hornet: Add a light skeleton data extractor scripts

This commit introduces scripts to extract eBPF instruction and map data
payloads from autogenerated light skeleton headers. These payloads can
then be used for signing a light skeleton binary using gen_sig.

> diff --git a/scripts/hornet/extract-insn.sh b/scripts/hornet/extract-insn.sh
> new file mode 100755
> index 000000000000..52338f057ff6
> --- /dev/null
> +++ b/scripts/hornet/extract-insn.sh
> @@ -0,0 +1,27 @@
[ ... ]
> +function usage() {
> +    echo "Sample script for extracting instructions"
> +    echo "autogenerated eBPF lskel headers"
> +    echo ""
> +    echo "USAGE: header_file"
> +    exit
> +}

Could this exit successfully (code 0) upon a usage error?

Calling exit without an argument defaults to the exit status of the previous
echo command, which is 0. If invoked incorrectly by a build system or
Makefile, this might fail to halt the build process.

This issue is also present in extract-map.sh and extract-skel.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

The autogenerated light skeletons explicitly include headers like <bpf/bpf.h>.

Since the script does not provide include paths (-I) to GCC or accept CFLAGS,
will the C preprocessor terminate with a fatal error if these headers are
not present in the standard system include paths, resulting in an empty
output file?

Additionally, is it possible for the unquoted $1 to cause word-splitting
or unintended command execution if a filename contains spaces or starts
with a compiler flag?

The grep pattern "opts_insn" appears to be overly broad. Could this match
not only the target byte array but also structural assignments generated by
bpftool, such as opts.insns_sz = sizeof(opts_insn) - 1?

Because the extra code is unquoted, it would split into multiple arguments.
When printf receives extra arguments without format specifiers, it repeatedly
reuses the format string, which in this case would cause it to duplicate
and corrupt the eBPF binary payload output.

Finally, since printf is called without a format specifier, could the
extracted eBPF payload be silently corrupted?

Bash's printf dynamically evaluates escapes. A byte sequence like \x25
(the % character) followed by \x73 (s) would be interpreted as %s and
replaced with an empty string, or \x64 (d) to 0, silently altering
the bytes and invalidating cryptographic signatures.

Could this be prevented by using a format string like
printf "%b" "$(...)"?

The gcc include, unquoted $1, and printf issues are also present in
extract-map.sh and extract-skel.sh.

> diff --git a/scripts/hornet/extract-map.sh b/scripts/hornet/extract-map.sh
> new file mode 100755
> index 000000000000..c309f505c623
> --- /dev/null
> +++ b/scripts/hornet/extract-map.sh
> @@ -0,0 +1,27 @@
[ ... ]
> +function usage() {
> +    echo "Sample script for extracting instructions"

This isn't a bug, but there is a copy-paste error in the usage message here.
Should this say "extracting map data" instead of "extracting instructions"?

[ ... ]
> +if [ $ARGC -ne $EXPECTED_ARGS ] ; then
> +    usage
> +else
> +    printf $(gcc -E $1 | grep "opts_data" | \
> +		 awk -F"=" '{print $2}' | sed 's/;\+$//' | sed 's/\"//g')
> +fi

Similar to extract-insn.sh, does the overly broad "opts_data" grep pattern
also match extra struct assignments like opts.data_sz = sizeof(opts_data) - 1
and cause payload duplication?

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