create-diff-object: correlate renumbered/uncorrelatable static local data and CSWTCH sections as a last resort - #1518
Open
benjamindonnachie wants to merge 1 commit into
Conversation
data and CSWTCH sections as a last resort gcc's numbering of some compiler-synthesized read-only data -- CSWTCH.N switch-dispatch tables, __compound_literal.N -- is not stable between two compilations of byte-identical source when the build *context* differs (a from-scratch build vs. an incremental, Kbuild-triggered rebuild of the same file later in the process, which is exactly what kpatch-build's two-pass original/patched flow does). Verified empirically: compiling the same unmodified file twice in a row, standalone, is byte-for-byte reproducible; it's specifically kpatch-build's from-scratch-then-incremental sequence that exposes the drift. kpatch_find_static_twin() correlates these by name and reference within the referencing function's section, and fails closed when the renumbering defeats that -- even when the file is provably unchanged. This is long-standing and still open upstream: dynup#767 (the exact file and symptom class reproduced here, cx2341x.c/CSWTCH.N), dynup#519, dynup#532, dynup#545. dynup#534 (merged 2015, "CSWTCH fix, take 2") addressed a different angle of the same general problem and is already folded into the more general is_special_static() mechanism -- it doesn't cover this case, since these are ordinary driver-defined static arrays (e.g. alc663_ssids, t4_reg_ranges) referenced *from* a CSWTCH section, not CSWTCH symbols themselves. Two last-resort fallbacks, both used only when the existing name/reference-based correlation already failed: 1. kpatch_find_static_twin_by_content(): correlate a static local by exact byte-for-byte section content, requiring the same base name (numeric suffix aside), type, and size. Content equality is the safety property here -- it can only let through cases the reference-based match would otherwise (safely) reject, never weaken the "no functional change" guarantee, since any genuine content difference still fails the comparison. The base-name requirement guards against unrelated tables coincidentally sharing identical bytes (e.g. short/terminator-only arrays). 2. kpatch_find_section_twin_by_symbol_ref(): CSWTCH.N sections can't be correlated by name at all -- every such section in a translation unit shares the same gcc-assigned base name, so name-based correlation is ambiguous by construction, not just numerically unstable. When a symbol referenced *from* such a section has already been correlated via the normal path (e.g. via some other, ordinarily-named function that also references it), use that as an anchor: the patched section whose relocations reference the symbol's twin is the section's twin. Reproduced and fixed while building a real cumulative EL9 5.14 kernel livepatch: this, together with the R_X86_64_32 and deferred find_local_syms fixes, took a build that previously failed deterministically on every attempt through to a working, loaded kpatch module. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1514 (the deepest of the four fixes tracked there).
gcc's numbering of some compiler-synthesized read-only data —
CSWTCH.Nswitch-dispatch tables,__compound_literal.N— is notstable between two compilations of byte-identical source when the
build context differs (a from-scratch build vs. an incremental,
Kbuild-triggered rebuild of the same file later in the process,
which is exactly what kpatch-build's two-pass original/patched flow
does). Verified empirically: compiling the same unmodified file
twice in a row, standalone, is byte-for-byte reproducible; it's
specifically kpatch-build's from-scratch-then-incremental sequence
that exposes the drift.
kpatch_find_static_twin()correlates theseby name and reference within the referencing function's section,
and fails closed when the renumbering defeats that — even when the
file is provably unchanged.
This is long-standing and still open upstream: #767 (the exact file
and symptom class reproduced here,
cx2341x.c/CSWTCH.N), #519,#532, #545. #534 (merged 2015, "CSWTCH fix, take 2") addressed a
different angle of the same general problem and is already folded
into the more general
is_special_static()mechanism — it doesn'tcover this case, since these are ordinary driver-defined static
arrays (e.g.
alc663_ssids,t4_reg_ranges) referenced from aCSWTCH section, not CSWTCH symbols themselves.
The fix
Two last-resort fallbacks, both used only when the existing
name/reference-based correlation already failed:
kpatch_find_static_twin_by_content(): correlate a staticlocal by exact byte-for-byte section content, requiring the same
base name (numeric suffix aside), type, and size. Content
equality is the safety property here — it can only let through
cases the reference-based match would otherwise (safely) reject,
never weaken the "no functional change" guarantee, since any
genuine content difference still fails the comparison. The
base-name requirement guards against unrelated tables
coincidentally sharing identical bytes (e.g.
short/terminator-only arrays).
kpatch_find_section_twin_by_symbol_ref(): CSWTCH.N sectionscan't be correlated by name at all — every such section in a
translation unit shares the same gcc-assigned base name, so
name-based correlation is ambiguous by construction, not just
numerically unstable. When a symbol referenced from such a
section has already been correlated via the normal path (e.g. via
some other, ordinarily-named function that also references it),
use that as an anchor: the patched section whose relocations
reference the symbol's twin is the section's twin.
Testing
Reproduced and fixed while building a real cumulative EL9 5.14
kernel livepatch (11 CVEs, base 5.14.0-687.25.1.el9_8 → target
5.14.0-687.38.1.el9_8). This, together with #1516 and #1517, took a
build that previously failed deterministically on every attempt
through to a working, loaded kpatch module — confirmed via
kpatch listafter installing the built RPM, covering all 11 target CVEs.Co-authored-by: Claude noreply@anthropic.com