Re: [PATCH v2 06/58] objtool/klp: Grow the klp test harness vocabulary
From: sashiko-bot@kernel.org
Date: 2026-09-14 06:41:09
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Regex boundary matching using `grep -qw` fails to enforce exact symbol name matches because GNU grep treats characters like `.` and `,` as non-word boundaries. -- commit cfe07e1e9cf1d7d13ada28e3a4fcdc25e8dd8672 Author: Song Liu [off-list ref] objtool/klp: Grow the klp test harness vocabulary This commit expands the objtool livepatch test harness vocabulary by adding assertions for both input and output objects. It validates properties like relocations, symbols, sections, and checksums produced by klp diff.
quoted hunk ↗ jump to hunk
diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh index 0a9da178f4dc8..40fe740821582 100644 --- a/tools/objtool/tests/lib.sh +++ b/tools/objtool/tests/lib.sh
[ ... ]
+count_input_symbols()
+{
+ in_symbols "$1" | awk -v n="$2" '$4 == "OBJECT" && $8 == n' | wc -l
+}
+
+# re_quote <string>
+#
+# A string as a literal basic regular expression. Nearly every name these
+# assertions match on contains a dot -- .text.target, .klp.rela.vmlinux -- and
+# an unescaped dot matches any character, so an assertion for one section can be
+# satisfied by a different one whose name merely lines up.
+re_quote() { printf '%s' "$1" | sed 's/[].[^$*\\/]/\\&/g'; }
+
+has_input_section() { in_sections "$1" | grep -q "[[:space:]]$(re_quote "$2")[[:space:]]"; }
+has_input_symbol() { in_symbols "$1" | grep -qw -- "$(re_quote "$2")"; }[Severity: Medium] Since GNU grep treats characters like dots as non-word boundaries (as accurately noted in the comment for count_input_symbols() above), won't this allow false positive matches? If a test checks for the symbol "target", couldn't this falsely match a symbol named ".text.target" or ".klp.tombstone.target"? [ ... ]
+assert_reloc_sym()
+{
+ section_relocs "$1" | grep -qw -- "$(re_quote "$2")" ||
+ fail "expected a relocation to '$2' in '$1'"
+}[Severity: Medium] Will this assertion mistakenly match base sections like ".text.unlikely" if $2 is ".text", because grep -qw relies on word boundaries that do not stop at dots?
+
+assert_no_reloc_sym()
+{
+ section_relocs "$1" | grep -qw -- "$(re_quote "$2")" &&
+ fail "unexpected relocation to '$2' in '$1'"
+ return 0
+}[Severity: Medium] Could this negative assertion falsely fail if a similarly-named dotted symbol is present? For example, failing to verify the absence of "foo" because "foo.isra.0" is present. [ ... ]
+assert_symbol()
+{
+ out_symbols | grep -qw -- "$(re_quote "$1")" ||
+ fail "expected symbol '$1' in output"
+}[Severity: Medium] Can this check pass falsely if only a tombstone or section symbol like ".klp.tombstone.foo" exists when verifying the presence of "foo"?
+
+assert_no_symbol()
+{
+ out_symbols | grep -qw -- "$(re_quote "$1")" &&
+ fail "unexpected symbol '$1' in output"
+ return 0
+}[Severity: Medium] Similar to assert_no_reloc_sym(), will this incorrectly fail if a cloned function like "foo.isra.0" exists when asserting the removal of "foo"? [ ... ]
+assert_livepatch_sym()
+{
+ out_symbols | grep -E 'OS ?\[0xff20\]' | grep -qw -- "$(re_quote "$1")" ||
+ fail "expected '$1' to be an SHN_LIVEPATCH symbol"
+}[Severity: Medium] Is this also prone to the same grep -qw boundary matching issue, potentially causing false passes on substring matches bounded by dots or other non-word characters? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260914062514.926997-1-song@kernel.org?part=6