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

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Unquoted `printf $(...)` causes binary data corruption and format string evaluation.
- [High] Overly broad `grep` patterns match struct assignments, leading to payload duplication.
- [Medium] Unquoted script argument `$1` allows command-line argument injection in `gcc`.
- [Low] Redundant and duplicate scripts create unnecessary maintenance burden.
--

commit 18ddbefea00184030f4d5a0c406b61b4fd5670fd
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 for use in signing.

> 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" | \
                        ^^

Could an unquoted $1 cause gcc to interpret filenames starting with a
hyphen as command-line options, or fail if the path contains spaces?

> +		 awk -F"=" '{print $2}' | sed 's/;\+$//' | sed 's/\"//g')

Does this overly broad grep match both the array declarations and
subsequent struct assignments?

If grep matches opts.insns = (void *)opts_insn;, the pipeline will extract
both the hex payload and the struct assignment, leading to payload duplication.

Also, can this printf $(...) construct corrupt the binary data?

If the extracted payload contains the byte 0x25 (%), printf will evaluate
it as a format specifier. For example, \x25\x64 (%d) will evaluate to 0. If
the payload starts with a hyphen, printf will interpret it as an option.

> 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
[ ... ]
> +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

This isn't a bug, but extract-insn.sh and extract-map.sh appear to duplicate
the functionality of extract-skel.sh, which cleanly accepts the target
field (insn or data) as a command-line parameter.

Could these redundant scripts be consolidated to avoid duplicating the
unquoted printf and grep issues across multiple files?

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