Re: [PATCH v5 36/58] objtool/klp: Add test for position-independent checksums

On Wed, Sep 16, 2026 at 12:01 PM [off-list ref] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] objtool/klp: test script comments mismatch C fixture implementation
--

commit 7722f94b411608f4277261adb44a750295859754
Author: Song Liu [off-list ref]
Date:   Wed Sep 16 18:43:29 2026 +0000

    objtool/klp: Add test for position-independent checksums

    A function that only moves has not changed, and its checksum must not move
    with it. Otherwise every patch reports as changed everything that shifted
    because something ahead of it grew.

quoted

diff --git a/tools/objtool/tests/generic/test-checksum-position.sh b/tools/objtool/tests/generic/test-checksum-position.sh
new file mode 100755
index 0000000000000..d05e28014e1e6
--- /dev/null
+++ b/tools/objtool/tests/generic/test-checksum-position.sh

@@ -0,0 +1,50 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0

[ ... ]

quoted

+# A jump or call without a relocation encodes its target as an offset from the
+# instruction.  Hashing those bytes makes the checksum change whenever anything
+# ahead of the function changes size -- so an unrelated edit elsewhere in the
+# file reports this function as changed too, and the patch grows to include it
+# and everything it references.  Nothing fails; the livepatch is just larger and
+# riskier than the patch it came from.
+#
+# Here the "patch" adds a function ahead of target() and changes nothing else.

[Severity: Low]
Is this comment accurate given that the C fixture uses __attribute__((aligned(64)))
instead of adding a new function?

It appears the C code in checksum_position.c explicitly states that PATCHED
changes the distance by aligning the functions rather than by inserting a
function between them.

quoted

+
+. "$(dirname "$0")/../lib.sh"
+
+setup
+
+# -fno-function-sections, or each function is at offset 0 of its own section
+# and target() never moves.
+build_pair checksum_position.c -fno-function-sections
+
+assert_input_symbol target
+
+# The fixture is only meaningful if target() actually moved.
+# What has to change is the displacement target's calls encode, and that is
+# the distance to callee() -- not target's own offset.  A compiler which put
+# the added function ahead of both would move target and leave the distance

[Severity: Low]
Does this explanation still apply?

Like the comment above, this seems to continue explaining the mechanism based
on an obsolete added function approach rather than the alignment approach
documented in the C fixture.

quoted

+# alone, and then the bytes are identical and the checksum matches for the
+# uninteresting reason.  Ask about the distance.
+sym_off()   # $1 object, $2 symbol
+{
+     in_symbols "$1" | awk -v n="$2" '$NF == n { print $2; exit }'
+}
+
+orig_t="$(sym_off orig.o target)";    orig_c="$(sym_off orig.o callee)"
+new_t="$(sym_off patched.o target)";  new_c="$(sym_off patched.o callee)"
+
+[ -n "$orig_t" ] && [ -n "$orig_c" ] && [ -n "$new_t" ] && [ -n "$new_c" ] ||
+     fail "target or callee missing from one of the objects"
+
+orig_gap=$(( 16#$orig_t - 16#$orig_c ))
+new_gap=$((  16#$new_t  - 16#$new_c ))
+[ "$orig_gap" != "$new_gap" ] ||
+     probe_skip "this compiler moved target() and callee() together;" \

[Severity: Low]
Should this skip message be updated as well?

It implies the functions were moved together by compiler reordering, as in the
added function approach, rather than reacting to the alignment directive used
in the C fixture.

Good catch here. We missed these comments/messages.

Josh,

Would you mind folding the following change in while applying the patches,
so that I don't have to spam the list again?

Thanks,
Song

diff --git c/tools/objtool/tests/generic/test-checksum-position.sh

w/tools/objtool/tests/generic/test-checksum-position.sh
index d05e28014e1e..5ae759e719eb 100755

--- c/tools/objtool/tests/generic/test-checksum-position.sh
+++ w/tools/objtool/tests/generic/test-checksum-position.sh

@@ -10,7 +10,9 @@
 # and everything it references.  Nothing fails; the livepatch is just

larger and
 # riskier than the patch it came from.
 #
-# Here the "patch" adds a function ahead of target() and changes nothing else.
+# Here the "patch" moves target() away from callee() and changes nothing else:
+# both are aligned to 64 in the patched build, which shifts them apart without
+# touching a byte of either.  See the fixture for why it is done that way.

 . "$(dirname "$0")/../lib.sh"

@@ -22,12 +24,11 @@ build_pair checksum_position.c -fno-function-sections

 assert_input_symbol target

-# The fixture is only meaningful if target() actually moved.
-# What has to change is the displacement target's calls encode, and that is
-# the distance to callee() -- not target's own offset.  A compiler which put
-# the added function ahead of both would move target and leave the distance
-# alone, and then the bytes are identical and the checksum matches for the
-# uninteresting reason.  Ask about the distance.
+# The fixture is only meaningful if the displacement target's calls encode
+# actually changed, and that is the distance to callee() -- not target's own
+# offset.  A compiler which shifted the two by the same amount would move
+# target and leave the distance alone, and then the bytes are identical and
+# the checksum matches for the uninteresting reason.  Ask about the distance.
 sym_off()   # $1 object, $2 symbol
 {
        in_symbols "$1" | awk -v n="$2" '$NF == n { print $2; exit }'

@@ -42,8 +43,8 @@ new_t="$(sym_off patched.o target)";

new_c="$(sym_off patched.o callee)"
 orig_gap=$(( 16#$orig_t - 16#$orig_c ))
 new_gap=$((  16#$new_t  - 16#$new_c ))
 [ "$orig_gap" != "$new_gap" ] ||
-       probe_skip "this compiler moved target() and callee() together;" \
-                  "the call displacement did not change"
+       probe_skip "this compiler kept target() and callee() the same
distance" \
+                  "apart; the call displacement did not change"

 assert_checksum_matches target