PXB-3804 : Fix --check-tables crash on broken FIL_PAGE_PREV sibling link#1762
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens xtrabackup --prepare --check-tables against multiple real-world InnoDB corruption patterns that previously could crash, hang, or attempt huge tablespace extensions, and adds regression tests (plus helper tooling) to exercise each scenario. It also extends --check-tables coverage to verify per-page checksums (and “future LSN” anomalies) for the system and undo tablespaces, which are not covered by the existing B-tree validation pass.
Changes:
- Add crash-safe/timeout-safe corruption handling in B-tree validation and related low-level helpers (record-chain validation, SDI validation, root fseg header validation, sibling/bounds checks).
- Add a system+undo tablespace checksum scan (plus “page LSN ahead of recovered system LSN” detection) as part of
--check-tables. - Add a new
ibd_tool.pyhelper and a comprehensivecheck_tablestest suite covering each fixed corruption scenario.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| storage/innobase/xtrabackup/test/suites/check_tables/table_types_and_options.sh | Refactors corruption injection to use the new mach_write_* helpers. |
| storage/innobase/xtrabackup/test/suites/check_tables/system_undo_checksum.sh | New non-debug test for system/undo checksum verification behavior. |
| storage/innobase/xtrabackup/test/suites/check_tables/system_undo_checksum_debug.sh | Debug-only test validating checksum pass via DBUG injection. |
| storage/innobase/xtrabackup/test/suites/check_tables/sibling_prev_link.sh | New regression test: broken FIL_PAGE_PREV must not crash. |
| storage/innobase/xtrabackup/test/suites/check_tables/sibling_next_out_of_bounds.sh | New regression test: corrupt FIL_PAGE_NEXT must not hang/extend. |
| storage/innobase/xtrabackup/test/suites/check_tables/sdi_record_header.sh | New regression test: SDI root record-header corruption must not crash. |
| storage/innobase/xtrabackup/test/suites/check_tables/root_fseg_space.sh | New regression test: root-page fseg header space-id mismatch must not crash. |
| storage/innobase/xtrabackup/test/suites/check_tables/root_fseg_page_no.sh | New regression test: nonsensical PAGE_LEVEL/inode lookup must not crash. |
| storage/innobase/xtrabackup/test/suites/check_tables/record_status_invalid.sh | New regression test: invalid REC_STATUS must be reported, not abort. |
| storage/innobase/xtrabackup/test/suites/check_tables/page_type_root.sh | New regression test: root FIL_PAGE_TYPE corruption must be handled. |
| storage/innobase/xtrabackup/test/suites/check_tables/page_type_all_levels.sh | New regression test: FIL_PAGE_TYPE corruption at all B-tree levels. |
| storage/innobase/xtrabackup/test/suites/check_tables/page_header_heap_top_nrecs.sh | New regression test: corrupt header fields PAGE_HEAP_TOP/PAGE_N_RECS. |
| storage/innobase/xtrabackup/test/suites/check_tables/node_ptr_no_extend.sh | New regression test: corrupt node-pointer child must not extend/hang. |
| storage/innobase/xtrabackup/test/suites/check_tables/injected_corruption_debug.sh | New debug-only DBUG injection coverage for multiple corruption modes. |
| storage/innobase/xtrabackup/test/suites/check_tables/incremental_future_lsn.sh | New test to ensure future-LSN checks don’t false-positive on incremental merge. |
| storage/innobase/xtrabackup/test/suites/check_tables/future_lsn_debug.sh | Debug-only test for deterministic future-LSN detection via injection. |
| storage/innobase/xtrabackup/test/suites/check_tables/father_child_mismatch.sh | New regression test covering invalid record header via user-index path. |
| storage/innobase/xtrabackup/test/run.sh | Adds check_tables suite to default runs and CLI help. |
| storage/innobase/xtrabackup/test/inc/ibd_tool.py | New Python helper to read/write/locate InnoDB page fields for tests. |
| storage/innobase/xtrabackup/test/inc/common.sh | Adds shell wrappers (mach_write_*, mach_read_*, page-finders) around ibd_tool.py. |
| storage/innobase/xtrabackup/src/xtrabackup.cc | Implements system+undo checksum scan; flushes buffer pools; adds SDI pre-validation; enables/disables no-extend gate during --check-tables. |
| storage/innobase/rem/rec.h | Declares rec_validate_page_chain() for XTRABACKUP. |
| storage/innobase/rem/rec.cc | Implements rec_validate_page_chain() for crash-safe record-chain validation. |
| storage/innobase/page/page0page.cc | Runs page-type/index-id/record-chain checks early under XTRABACKUP to prevent aborts in rec_get_offsets(). |
| storage/innobase/include/rem0rec.ic | Disables ut_ad() assertions in rec_get_next_offs() under XTRABACKUP so validation can fail gracefully. |
| storage/innobase/include/fsp0fsp.h | Declares fseg_root_header_validate() for XTRABACKUP. |
| storage/innobase/include/fil0fil.h | Declares fil_check_tables_no_extend gate for XTRABACKUP reads. |
| storage/innobase/include/api0api.h | Declares ib_sdi_validate() for XTRABACKUP SDI index validation. |
| storage/innobase/fsp/fsp0fsp.cc | Implements fseg_root_header_validate() using fseg_inode_try_get() and bounds checks. |
| storage/innobase/fil/fil0fil.cc | Implements/uses fil_check_tables_no_extend to refuse out-of-bounds auto-extend during validation. |
| storage/innobase/btr/btr0btr.cc | Adds multiple XTRABACKUP corruption guards (sibling bounds, father-node ptr handling, root fseg validation paths). |
| storage/innobase/api/api0api.cc | Implements ib_sdi_validate() to validate SDI B-tree before SDI-driven dictionary load. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add the shared test scaffolding used by the --check-tables regression tests
that follow, so each later fix commit can carry its tests without restating
the framework:
* inc/ibd_tool.py: one module holding the on-disk page logic, with accessors
named after their InnoDB C counterparts (and the same offsets) so they read
like the engine -- mach_read_from_2/4/8, fil_page_get_type /
fil_page_index_page_check, page_get_page_no, fil_page_get_prev/next/lsn,
btr_page_get_level / btr_page_get_index_id, page_header_get_field,
page_get_n_recs, page_dir_get_n_slots, rec_get_next_offs,
fsp_flags_get_page_size. Page constants are defined once. It is also a
self-documenting standalone tool (python3 ibd_tool.py -h) with subcommands
for terminal use: summary / header / scan / page-size / read / write /
flags / encryption / space-id / space-ids and the B-tree navigation helpers.
* inc/common.sh: thin shell wrappers over that module, forwarding an optional
exported PAGE_SIZE override:
- get_page_size logical page size from the FSP header
- mach_read_2 / mach_read_4, mach_write_2/4/8
big-endian read/write of a page field at
(page_no * page_size + offset)
- find_index_page first INDEX page at a given B-tree level
- find_leftmost_node_ptr root page + offset of the leftmost node
pointer's child page number
- find_clustered_pages_by_level
a clustered-index page at levels 0/1/2
- find_first_user_rec_origin
first user record origin via the infimum
"next" link
* run.sh: register the check_tables suite in the default test/suite lists
and the -s usage text.
No product code and no tests yet; subsequent commits add their tests under
suites/check_tables/ using these helpers.
https://perconadev.atlassian.net/browse/PXB-3804 Problem: -------- xtrabackup --prepare --check-tables hung indefinitely when a B-tree page carried an out-of-bounds page reference -- a corrupt FIL_PAGE_NEXT / FIL_PAGE_PREV sibling link, or a corrupt node-pointer child page number: [MY-012144] posix_fallocate(): Failed to preallocate data for file ./test/test_users.ibd, desired size 61209453068288 bytes. A value such as 0xDEADBEEF is treated as a real page number; the buffer pool then tries to extend the tablespace to cover it (~61 TB), posix_fallocate fails with EFBIG, and the extend path retries forever. This affects release builds too -- the extension is not gated behind any debug assertion. Fix: ---- Two complementary, read-only defenses: 1. btr_page_no_in_bounds(space_id, page_no) verifies a page number read from page data is below the current tablespace size (fil_space_get_size). In btr_validate_level(), under XTRABACKUP, both sibling links are checked against the bound right after they are read and before any are followed; if either is out of range the index is reported corrupt and traversal stops instead of fetching the bogus page. 2. A read-only "no tablespace extend" gate (fil_check_tables_no_extend, an atomic) in Fil_shard::do_io(). --check-tables is strictly read-only, so any out-of-bounds page number reaching the IO layer (e.g. a node-pointer child page number, which btr_validate descends without a bounds check) is corruption, not a short-copied file needing redo-apply extension (PXB-1819). The gate refuses to grow the file and returns an error so the read fails fast. xtrabackup.cc sets the flag around the check-tables phase. Server builds are unchanged. Tests: ------ suites/check_tables/sibling_next_out_of_bounds.sh corrupts FIL_PAGE_NEXT of a leaf page to 0xDEADBEEF and asserts --check-tables completes (no hang), performs no absurd posix_fallocate, reports corruption, and exits non-zero (defense 1). suites/check_tables/node_ptr_no_extend.sh corrupts the leftmost node-pointer's child page number to 0xDEADBEEF (a path the sibling bounds check does NOT cover) and asserts the backup .ibd is not extended and --check-tables does not hang (defense 2). Removing the gate makes this test hang on the ~61 TB posix_fallocate.
https://perconadev.atlassian.net/browse/PXB-3804 Problem: -------- During --check-tables, btr_validate_level() cross-checks that a page's right sibling links back (its FIL_PAGE_PREV points to the current page). When that back-link is broken, the only handling was a debug-build ut_ad() assertion (guarded by a DBUG_EXECUTE_IF escape hatch); on a release build the inconsistency was effectively ignored and traversal continued following sibling links that are now untrustworthy. Fix: ---- Under XTRABACKUP, on a broken back-link, report corruption, set right_page_no = FIL_NULL and goto node_ptr_fails to stop traversing this level instead of chasing the remaining (untrustworthy) sibling links. The debug-only assertion path is kept for non-XTRABACKUP builds via '#elif defined(UNIV_DEBUG)'. Tests: ------ suites/check_tables/sibling_prev_link.sh (debug build: uses the check_table_break_sibling_link DBUG injection) breaks the back-link and asserts --check-tables reports corruption and exits non-zero without aborting.
https://perconadev.atlassian.net/browse/PXB-3804 Problem: -------- A corrupt root-page file-segment header (FSEG_HDR_SPACE naming the wrong tablespace, or a bad FSEG_HDR_PAGE_NO / in-page offset) aborted --check-tables: btr_root_block_get() asserts the header via btr_root_fseg_validate() under UNIV_BTR_DEBUG, and the inode lookup (fseg_inode_get -> ut_a(inode)) aborts on a bad page number -- a crash rather than a graceful corruption report. Fix: ---- Make the root file-segment headers validated and reported, not asserted, under XTRABACKUP: * btr_root_fseg_validate() is compiled under XTRABACKUP and returns false (instead of ut_a) when FSEG_HDR_SPACE/offset are wrong. * btr_root_block_get() no longer asserts the headers under XTRABACKUP (#if defined(UNIV_BTR_DEBUG) && !defined(XTRABACKUP)); validation moves to btr_validate_index(). * btr_validate_index() validates both root headers (FSEG_HDR_SPACE/offset via btr_root_fseg_validate, then FSEG_HDR_PAGE_NO / inode via the new fseg_root_header_validate() in fsp0fsp.cc) before the inode lookup, and reports corruption + returns false on failure. fseg_root_header_validate() (fsp0fsp.cc/.h) safely checks the segment-inode page number and offset without asserting. Tests: ------ suites/check_tables/root_fseg_page_no.sh corrupts FSEG_HDR_PAGE_NO and root_fseg_space.sh corrupts FSEG_HDR_SPACE on the root page; both assert --check-tables reports corruption and exits non-zero without aborting.
…heck-tables https://perconadev.atlassian.net/browse/PXB-3804 Problem: -------- During --check-tables, btr_validate_level() locates each page's father node pointer via btr_page_get_father_node_ptr_for_validate(). When a father node pointer's child page number does not match the child page (B-tree corruption), the old code either ran page_rec_print() on records it was about to treat as corrupt, or dereferenced offsets the lookup never produced -- aborting instead of reporting corruption. Fix: ---- * btr_page_get_father_node_ptr_func(): under XTRABACKUP, when the child page number does not match during validation (BTR_CONT_SEARCH_TREE), report the mismatch and return nullptr instead of running the page_rec_print() path. * btr_validate_level(): at each btr_page_get_father_node_ptr_for_validate() call (current page, and the right sibling), treat a nullptr result as corruption -- report it, set right_page_no = FIL_NULL and goto node_ptr_fails -- instead of dereferencing the missing offsets. Tests: ------ suites/check_tables/father_child_mismatch.sh corrupts a node pointer's child page number so the father/child relationship breaks, and asserts --check-tables reports corruption and exits non-zero without aborting. (The leaf page to corrupt is located dynamically via the common.sh helpers.)
|
https://pxb.cd.percona.com/view/PXB%208.1/job/percona-xtrabackup-8.1-multijob/152/ after the ASAN fixes |
… in --check-tables https://perconadev.atlassian.net/browse/PXB-3804 Problem: -------- Several --check-tables corruptions were caught only by debug-only paths or not at all, so a release xtrabackup crashed, aborted, or silently passed: * a B-tree page whose FIL_PAGE_TYPE was corrupted (INDEX -> ALLOCATED) on the root or any level crashed or was silently skipped; * an SDI page with a corrupt record header crashed while the SDI was loaded; * a record with an out-of-range REC_STATUS aborted in rec_get_offsets() (default: ut_error); * a page whose stored index id did not match the index being validated was not consistently reported. Fix: ---- Consolidate all within-page validation into page_validate(), the single release-safe per-page validator every index page (user and SDI) flows through: * fil_page_index_page_check() -- the page must be an INDEX page; * btr_page_get_index_id(page) == index->id -- the page belongs to the index; * rec_validate_page_chain() (rem/rec.cc) -- walk the record chain via the "next" pointers, confirming every offset stays within the page and every record status is ORDINARY/NODE_PTR/SUPREMUM, bounded by a maximum record count so a corrupt header cannot send the walk off-page or loop. rec_get_next_offs() no longer asserts under XTRABACKUP. Non-index (LOB) and REDUNDANT pages are skipped. btr_validate_level() routes free/invalid pages through page_validate() and stops the level on failure (goto node_ptr_fails) instead of scattered checks. SDI validation is unified through the normal B-tree path: ib_sdi_validate() (api0api.cc) opens the SDI and runs btr_validate_index() over it, so the SDI is checked by the same page_validate() logic; xtrabackup.cc calls it per space. Tests: ------ * page_type_root.sh / page_type_all_levels.sh -- corrupt FIL_PAGE_TYPE on the root and at every B-tree level; * sdi_record_header.sh -- corrupt an SDI page record header; * record_status_invalid.sh -- corrupt a record's REC_STATUS to an out-of-range value (only rec_validate_page_chain catches this; without it the run aborts at rec.cc ut_error); * page_header_heap_top_nrecs.sh -- corrupt PAGE_HEAP_TOP / PAGE_N_RECS (validated by page_simple_validate_new(), reached via this consolidation); * table_types_and_options.sh / injected_corruption_debug.sh -- positive coverage across table/index/tablespace types and DBUG-injected corruptions. Each asserts --check-tables reports corruption with a non-zero exit and no crash, abort, or silent pass.
…-tables https://perconadev.atlassian.net/browse/PXB-3807 Problem: -------- --check-tables only walks fsp_is_ibd_tablespace() B-trees, so the system tablespace (ibdata*, which also holds the legacy rollback segments) and the undo tablespaces -- rollback segments, change buffer, undo logs, allocation/ free pages -- received no integrity check at all. Fix: ---- Add xb_checksum_system_undo_spaces(), a single-threaded, engine-native pass run at the top of the check-tables phase over space 0 (TRX_SYS_SPACE) and every undo::spaces entry. Each page is read with fil_io() into a private aligned buffer (ut_unique_ptr) -- NOT into the buffer pool -- which maps the page to the right ibdata file and decrypts/decompresses it, while deliberately bypassing buf_page_io_complete()'s abort-on-corruption policy. Each page is then run through BlockReporter::is_corrupted() (the same checksum core innochecksum uses) and corruption is reported gracefully (non-zero exit), not aborted. Legacy doublewrite pages in the system tablespace are skipped. The buffer pool is flushed first (buf_flush_sync_all_buf_pools()) so the raw on-disk image read here equals the recovered state -- post-recovery undo/ trx-sys/purge init can leave system pages dirty, which would otherwise read as a stale/torn page and report a false positive. Flushing does not advance the redo checkpoint, so it is safe under --apply-log-only. Tests: ------ suites/check_tables/system_undo_checksum.sh corrupts a cold page of an undo tablespace and of ibdata1 and asserts --check-tables reports a checksum mismatch and exits non-zero, with a clean-run control; system_undo_checksum_ debug.sh uses the check_system_inject_corruption DBUG point plus a clean control.
https://perconadev.atlassian.net/browse/PXB-3807 Problem: -------- The system/undo checksum pass confirms a page's checksum is internally consistent, but a page can be checksum-valid yet carry an FIL_PAGE_LSN ahead of the LSN we recovered to -- e.g. an incomplete or over-applied backup, the wrong redo, or a corrupt LSN field. Such a page was accepted silently. Fix: ---- Extend xb_checksum_system_undo_spaces(): for every checksum-valid page, read FIL_PAGE_LSN and compare it to the recovered system LSN (log_get_lsn). A page whose LSN is ahead is reported and fails the check. A page that failed the checksum is skipped (its LSN field is unreliable). Unlike innochecksum, this runs inside the engine after recovery, so the recovered system LSN is known. Tests: ------ suites/check_tables/future_lsn_debug.sh uses the check_system_inject_future_lsn DBUG point to force a page LSN past the system LSN and asserts --check-tables reports it and exits non-zero. incremental_future_lsn.sh is a regression guard proving an incremental-merge prepare does not raise a false future-LSN report (the .delta pages' future LSNs are caught up by redo before check-tables runs).
…ix ASAN leaks)
Problem:
--------
On an ASAN/LeakSanitizer build, every negative --check-tables test that detects
corruption, reports it, and exits non-zero failed: LeakSanitizer reported ~6 MB
"definitely lost" (e.g. 5 x 1,179,504 bytes -- the trx pools of 8191 trx_t --
plus the buffer pool, fil system, dict and latch registry) and aborted
(SIGABRT), which the tests see as a crash. 10 of the build-176 asan-axis
failures were exactly these check_tables tests.
Analysis:
---------
xtrabackup_prepare_func()'s success path tears InnoDB down via innodb_end() ->
srv_shutdown() (which also closes sync_check / os_thread). The corruption path
("Table check failed") instead does `goto error_cleanup`, and error_cleanup
exits WITHOUT any InnoDB shutdown -- so the whole InnoDB startup footprint
leaks. Harmless on a normal build (the OS reclaims at exit); on ASAN it is a
leak + abort. A pre-existing latent gap that the new check_tables negative
tests are the first to exercise.
Fix:
----
1. error_cleanup: when InnoDB came fully up (innodb_inited), call innodb_end()
before exiting. srv_shutdown() assumes a completed init, so the innodb_inited
guard ensures a partial-startup failure is left untouched (we only free what
was fully allocated). innodb_end() does the complete, balanced teardown
(including sync_check_close()/os_thread_close()); nothing else is needed.
2. Fil_shard::do_io() read-only no-extend gate: call complete_io(file, req_type)
before returning DB_ERROR. prepare_file_for_io() had incremented
file->n_pending_ios; the early return leaked that reference, which is now
load-bearing -- the InnoDB shutdown added in (1) asserts n_pending_ios == 0
during fil_close, so node_ptr_no_extend would otherwise abort on shutdown.
(Thanks to the GitHub Copilot PR review for flagging the missing
complete_io().)
Verified by reproducing locally with a -DWITH_ASAN=1 -DWITH_DEBUG=1 build:
all 17 check_tables tests pass under ASAN with 0 LeakSanitizer reports
(previously 10 failed with the ~6 MB leak + abort), and the normal debug /
release builds remain green (17/17 ; 13 + 4 skipped).
… tooling
Test framework only (no product change). Extends the check_tables helpers so
the corruption tests can recompute a valid checksum and locate the leftmost
leaf without inline Python:
* ibd_tool.py: ut_crc32 (CRC-32C / Castagnoli, pure-Python -- no external
dependency such as crcmod) and buf_calc_page_crc32, mirroring InnoDB's
checksum.cc; verified byte-exact against clean pages and the standard
ut_crc32("123456789") == 0xe3069283 vector. New subcommands: fill,
update-checksum, and leftmost-leaf (leftmost level-0 page + its sibling).
* common.sh: fill_page_range, update_page_checksum and find_leftmost_leaf.
https://perconadev.atlassian.net/browse/PXB-3804
…heck-tables A partial/torn page write can leave PAGE_LEVEL above BTR_MAX_NODE_LEVEL. During --check-tables this aborted xtrabackup: btr_validate_level()'s descent reads the level with btr_page_get_level() (ut_ad on debug) and, on release, mistakes the page for an internal node and parses its records as node pointers (rec.cc "default: ut_error"). Stop btr_page_get_level() from asserting on the level under XTRABACKUP and add btr_page_level_is_sane() beside it. The descent guards (root and each child) and page_validate() use it to reject an out-of-range PAGE_LEVEL and report corruption instead of aborting. Test: partial_page_write (checksum-off and recomputed-valid-checksum variants). https://perconadev.atlassian.net/browse/PXB-3804
…n --check-tables btr_validate_level() compares the last record of the current page with the first record of its right sibling to verify cross-page key ordering, but the right sibling is only run through page_validate() on the NEXT iteration. A sibling with a corrupt REC_STATUS therefore reached rec_get_offsets(right_rec) and aborted (rec.cc "default: ut_error") before it was ever validated. Validate the right sibling's page type and record chain before parsing its first record; report corruption and stop the level scan otherwise. Test: sibling_record_status. https://perconadev.atlassian.net/browse/PXB-3804
…k-tables btr_validate_index() reads the root level up front with ulint n = btr_page_get_level(root) and loops btr_validate_level(n-i). A corrupt root PAGE_LEVEL would make n huge and spin btr_validate_level() up to 65536 times. This read is above btr_validate_level(), so its own level checks do not cover it; reuse btr_page_level_is_sane() to reject it and fail the index check cleanly. Test: root_level_corrupt. https://perconadev.atlassian.net/browse/PXB-3804
…a visited-page set btr_validate_level() scans a level left to right via FIL_PAGE_NEXT. On a self-consistent cycle the back-link and bounds checks pass, the key-order check reports but does not stop, and check-tables runs with trx=nullptr so trx_is_interrupted() never fires -- so --check-tables hung forever. Track the pages already visited on the level in a std::unordered_set: a page can appear only once per level, so revisiting one means the sibling links loop. Only the page number is recorded, and only when a page becomes the current page (once per left->right step), so re-reads/re-latches during the scan are never re-inserted, and the root re-descended for other subtrees is a separate per-level pass with its own set. Detection is at the first revisit (one loop around). Test: sibling_link_cycle. https://perconadev.atlassian.net/browse/PXB-3804
…inter cycles btr_validate_level()'s descent loop (while level != btr_page_get_level(page)) has no iteration bound and does not require the level to strictly decrease; btr_descent_level_is_sane() only checks the level is in range. A node pointer whose child-page-number is corrupted to point sideways/upward (e.g. back to the root) made the descent loop forever (check-tables runs with trx=nullptr, so trx_is_interrupted() never fires). Cap the descent at BTR_MAX_NODE_LEVEL steps -- a well-formed descent moves one level down per step -- and report corruption if exceeded. Test: descent_nodeptr_cycle. https://perconadev.atlassian.net/browse/PXB-3804
0437036 to
b3379cb
Compare
PXB-3807 : Verify system and undo tablespace checksums during --check-tables
The commits in this PR mainly enhance the corruption checks. Added test cases for each fixed corruption scenario.