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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] extract-skel.sh and extract-map.sh fail with a usage error when extracting empty payloads.
- [High] The scripts strictly forbid passing include paths (-I) to the C preprocessor.
- [Medium] Unquoted variables in the scripts allow command injection and fail on filenames with spaces.
- [Medium] Improper use of printf with unquoted command substitution pollutes stderr and relies on undocumented behavior.
- [Medium] Trailing whitespace in the header file bypasses semicolon stripping, corrupting the payload.
- [Low] The usage function exits with a success status (0) on invalid arguments.
- [Low] Usage text in extract-map.sh incorrectly describes its functionality.
- [Low] Grammar mistakes in the commit message.
--

commit 4a15a4a590708cdca529a456f67bfad392cfd02a
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

hornet: Add a light skeleton data extractor scripts

These script eases light skeleton development against Hornet by
generating a data payloads which can be used for signing a light
skeleton binary using gen_sig.

> hornet: Add a light skeleton data extractor scripts
> 
> These script eases light skeleton development against Hornet by
> generating a data payloads which can be used for signing a light
> skeleton binary using gen_sig.

This isn't a bug, but there is a grammar mistake in the commit message.
"These script eases" and "a data payloads" mix singular and plural forms.

> 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
> @@ -0,0 +1,27 @@
> +#!/bin/bash

[ ... ]

> +function usage() {
> +    echo "Sample script for extracting instructions"
> +    echo "autogenerated eBPF lskel headers"
> +    echo ""
> +    echo "USAGE: header_file"
> +    exit
> +}

Could this cause build scripts to silently continue even when arguments are
missing? Calling exit without an argument defaults to the exit status of
the last command, which is echo in this case, returning a success status
of 0 instead of an error code.

> +ARGC=$#
> +
> +EXPECTED_ARGS=1
> +
> +if [ $ARGC -ne $EXPECTED_ARGS ] ; then
> +    usage
> +else

Will this strict argument limit break in standard kernel build environments?

BPF light skeletons include <bpf/skel_internal.h>, which typically requires
passing include paths like -Itools/include to gcc -E. Enforcing exactly one
argument prevents passing any such flags to the preprocessor.

> +    printf $(gcc -E $1 | grep "opts_insn" | \

Could the unquoted $1 cause gcc to fail if the header file path contains
spaces?

Is there a risk of printf throwing a usage error and failing the build if a
BPF object has no maps? If bpftool completely skips generating the data
array, the grep command returns nothing, and the unquoted command
substitution evaluates to zero arguments. A call to printf with no arguments
will throw a bash usage error to stderr and exit with a non-zero status.

Also, since grep might match multiple lines such as the array declaration
and opts.insns_sz = sizeof(...), the unquoted substitution will split them
into multiple words. Does this cause printf to use the first word as a
format string and issue warnings to stderr about ignoring excess arguments?

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

Does the sed 's/;\+$//' command safely handle carriage returns or trailing
spaces?

If the generated C header contains whitespace after the semicolon, the line
ending anchor $ will fail to match, leaving the semicolon in the string
and potentially corrupting the extracted binary payload.

> 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
> @@ -0,0 +1,27 @@
> +#!/bin/bash

[ ... ]

> +function usage() {
> +    echo "Sample script for extracting instructions"
> +    echo "autogenerated eBPF lskel headers"

This isn't a bug, but should this usage text mention that it extracts map
data instead of instructions?

> 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

Is it safe to pass $2 directly into the unquoted grep command here? If the
field argument contains spaces or shell characters like quotation marks or
pipes, it could result in command injection.

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