From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id E7F2A1FF09B for ; Mon, 09 Nov 2026 14:39:18 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9D7B42140D; Mon, 09 Nov 2026 14:39:18 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup] fix #8186: pxar: fix off-by-one error in dynamic entry range end check Date: Mon, 9 Nov 2026 14:39:00 +0100 Message-ID: <20261109133900.426552-1-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794231553748 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.434 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: XTREWN54YWSUBV6XYU3ZE6DHCGBJBVQJ X-Message-ID-Hash: XTREWN54YWSUBV6XYU3ZE6DHCGBJBVQJ X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: With change-detection-mode metadata, the pxar archiver tires to reuse unchanged files instead of re-encoding them. This happens by comparing the file metadata against the matching file from the previous metadata archive. If reusable, the range of dynamic entries to reuse from the previous payload archive are determined. To avoid reusing many partial chunks with only little actual payload, the archiver uses a lookahead cache and keeps a continuous range of reusable dynamic entries in order to decide whether to reuse or re-encode cached entries. The check for dynamic entries included in the files payload range was however off-by-one, checking only if the end of the dynamic entry is smaller than the range end. This however leads to unnecessary inclusion of the next entry in case the end was equal, with the consequence that files which potentially could have been reused might get re-encoded, if the payload padding exceeds the padding threshold. Both, dynamic entries end and range end are exclusive, so check for equality in order to only include strictly required dynamic entries. Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8186 Reported-by: Signed-off-by: Christian Ebner --- pbs-client/src/pxar/create.rs | 44 ++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs index 4d25341a7..b1266b1fc 100644 --- a/pbs-client/src/pxar/create.rs +++ b/pbs-client/src/pxar/create.rs @@ -1468,7 +1468,7 @@ fn lookup_dynamic_entries( }; indices.push(reusable_dynamic_entry); - if range.end < end { + if range.end <= end { padding_end = end - range.end; break; } @@ -2187,6 +2187,48 @@ mod tests { let _ = std::fs::remove_file(&path); } + #[test] + fn lookup_dynamic_entries_range_end_check() { + use pbs_datastore::dynamic_index::{DynamicIndexReader, DynamicIndexWriter}; + + let testdir = create_testdir("lookup_dynamic_entries_range_end_check") + .expect("Should be able to create test dir"); + let path = testdir.join("lookup_dynamic_entries.didx"); + let _ = std::fs::remove_file(&path); + + let mut writer = DynamicIndexWriter::create(&path).unwrap(); + writer.add_chunk(1024, &[0xAAu8; 32]).unwrap(); + writer.add_chunk(2048, &[0xBBu8; 32]).unwrap(); + writer.add_chunk(3072, &[0xCCu8; 32]).unwrap(); + let _csum = writer.close().unwrap(); + + let reader = DynamicIndexReader::open(&path).unwrap(); + for (range, count, start_padding, end_padding) in [ + (0..1024, 1, 0, 0), + (16..1024, 1, 16, 0), + (0..1000, 1, 0, 24), + (1024..2048, 1, 0, 0), + (0..2048, 2, 0, 0), + (0..2049, 3, 0, 1023), + ] { + let (indices, start, end) = super::lookup_dynamic_entries(&reader, &range).unwrap(); + assert_eq!( + indices.len(), + count, + "incorrect chunk count: got {}, expected {count} for {range:?}", + indices.len(), + ); + assert_eq!( + start, start_padding, + "incorrect start padding: got {start}, expected {start_padding} for {range:?}" + ); + assert_eq!( + end, end_padding, + "incorrect end padding: got {end}, expected {end_padding} for {range:?}" + ); + } + } + /// Inversions that span cache ranges must be caught by the global tracking; a per-range /// implementation would have reset on flush and accepted the second offset, after which /// the encoder strict check would still abort the backup. -- 2.47.3