From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 60E1C27CCE0 for ; Fri, 8 May 2026 02:35:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778207735; cv=none; b=lXnvHeZTOm1MbInyI73bxz72/qV9M8si5rd7xlLzF1Hw0rNaRHEOxzwfnyUMl6cXJQfsCWI6279v69gBVyiFuQct0TURbffkEUgrYgEIwZOu+FtoizCC0ao3SSntl/Gr7Xl/SSooPbfRoV0AP3OJEOqkywVXfAVEsCjrzzDy1QI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778207735; c=relaxed/simple; bh=czRmnwIS0mbnHZkE1dIBK/YGkH4nY+vXVZxXmR0d3ys=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=c8J/5bjKUiMbrzukrXR5FY3deoHinVrrT/B97Ag9kbtQKS8gkIF4hxd5dEw7wKysrXPY+wpvjd0hm/neiMVKEZzHAqVDonWquXydiUpIiW7kDCtdQ3GO2XqMl2z+T70GkZ/4NthtSc2cXbAZISEiilYO4vRowDEFzNnDqrvXui4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=sdkMz1+w; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="sdkMz1+w" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E00F4C2BCB2; Fri, 8 May 2026 02:35:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778207735; bh=czRmnwIS0mbnHZkE1dIBK/YGkH4nY+vXVZxXmR0d3ys=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=sdkMz1+wwCJeIkKrtGfwRbmB2P03nKcM3Wtu3CyNmjwgCwZYK9SAPK2XTi1hZ7h/H wcaMFLH878/1zSeCJZrD8fd8i3xZSDxSwUe+NGgG6d7fAGdD4KM4dSXRl+xMi9jgT9 vuCZGxXnJ7mspGTVj1JTAGuenUNxsR3PIUH9k2M29SrpsRdKJH25WyMOOsP/iJn+j1 Ne4bWpByRECLMxPEVmyevkiMUMCbVW0QBVdv7ylvZJ9WzfT/ySEAbgP2VXcYFVhyNA nwwAeXg4jnjSeLie02SKXS5Sem8VYrypMbYak3g2g9/GoIkn91JhiZ7YPy9olcPPge t5rn1xaHxaONw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v7 08/10] hornet: Add a light skeleton data extractor scripts Reply-To: sashiko@lists.linux.dev To: "Blaise Boscaccy" Cc: bpf@vger.kernel.org In-Reply-To: <20260507191416.2984054-9-bboscaccy@linux.microsoft.com> References: <20260507191416.2984054-9-bboscaccy@linux.microsoft.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 08 May 2026 02:35:34 +0000 Message-Id: <20260508023534.E00F4C2BCB2@smtp.kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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 ex= tracting empty payloads. - [High] The scripts strictly forbid passing include paths (-I) to the C pr= eprocessor. - [Medium] Unquoted variables in the scripts allow command injection and fa= il on filenames with spaces. - [Medium] Improper use of printf with unquoted command substitution pollut= es stderr and relies on undocumented behavior. - [Medium] Trailing whitespace in the header file bypasses semicolon stripp= ing, corrupting the payload. - [Low] The usage function exits with a success status (0) on invalid argum= ents. - [Low] Usage text in extract-map.sh incorrectly describes its functionalit= y. - [Low] Grammar mistakes in the commit message. -- commit 4a15a4a590708cdca529a456f67bfad392cfd02a Author: Blaise Boscaccy 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 >=20 > 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=3D$# > + > +EXPECTED_ARGS=3D1 > + > +if [ $ARGC -ne $EXPECTED_ARGS ] ; then > + usage > +else Will this strict argument limit break in standard kernel build environments? BPF light skeletons include , 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 =3D 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"=3D" '{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"=3D" '{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. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260507191416.2984= 054-1-bboscaccy@linux.microsoft.com?part=3D8