ddidderr/zstd-rs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compress): guard dictionary repcode coordinates

Browse Source 
Attached-dictionary matchers derive dictionary and prefix pointers from
32-bit repcode coordinates.  A stale offset in the Rust double-fast
post-match path could wrap the subtraction to a large index, while the
C-compatible overlap predicate's intentional unsigned arithmetic then
classified that index as safe.  Dictionary training consequently reached an
invalid read in the multithreaded FastCover workers.

Centralize the attached-dictionary coordinate check before pointer formation:
reject disabled or wrapped offsets, keep repcodes no later than the current
position, require four-byte dictionary matches to remain in the translated
dictionary interval, and preserve the active-prefix coordinate region.  Apply
the same checked overlap semantics in the optimal parser, where the current
position is available to reject wrapped repcodes.  Focused tests cover the
crashing coordinate, dictionary boundaries, prefix boundaries, and valid
active-prefix repcodes.

Test Plan:
- `cargo test --manifest-path rust/Cargo.toml --lib zstd_double_fast::tests` -- passed (4 tests)
- `cargo test --manifest-path rust/Cargo.toml --lib zstd_opt::tests::index_overlap_check` -- passed (2 tests)
- `cargo test --manifest-path rust/Cargo.toml --all-targets` -- passed (400 tests)
- `cargo clippy --manifest-path rust/Cargo.toml --lib -- -D warnings` -- passed
- `rustfmt +nightly --edition 2021 rust/src/zstd_double_fast.rs rust/src/zstd_opt.rs --check` -- passed
- `make -C programs -j2 zstd` -- passed
- `./programs/zstd --train -B2K tests/tmpCorpusHighCompress -o /tmp/zstd-rust-dict-high-fixed` -- passed
- `make -C tests -j2 test-zstd` -- passed
- Full `cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings` remains blocked by the pre-existing `manual_repeat_n` lint at `rust/src/zstd_compress.rs:2351`.
This commit is contained in:
ddidderr
2026-07-18 17:13:27 +02:00
parent 5dee449036
commit 98cca9aa0f
2 changed files with 223 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
rust/src/zstd_double_fast.rs
+196 -66
View File
@@ -235,7 +235,41 @@ fn lowest_match_index(low_limit: u32, loaded_dict_end: u32, curr: u32, window_lo
#[inline]
fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32) -> bool {
prefix_lowest_index.wrapping_sub(1).wrapping_sub(rep_index) >= 3
/*
* Keep the C predicate's two valid coordinate regions without relying on
* unsigned subtraction to classify an already-invalid index. The
* attached-dictionary coordinate guard below additionally bounds prefix
* repcodes by the current position.
*/
rep_index >= prefix_lowest_index
|| rep_index
.checked_add(4)
.is_some_and(|rep_end| rep_end <= prefix_lowest_index)
}
#[inline]
fn attached_rep_index(
prefix_lowest_index: u32,
dict_start_index: u32,
dict_index_delta: u32,
current: u32,
advance: u32,
offset: u32,
) -> Option<u32> {
if offset == 0 {
return None;
}
let rep_index = current.checked_add(advance)?.checked_sub(offset)?;
if rep_index > current || !index_overlap_check(prefix_lowest_index, rep_index) {
return None;
}
if rep_index < prefix_lowest_index {
let dict_rep_start = dict_start_index.checked_add(dict_index_delta)?;
if rep_index < dict_rep_start {
return None;
}
}
Some(rep_index)
}
#[inline]
@@ -774,6 +808,7 @@ unsafe fn post_dict_match(
base: *const u8,
dict_base: *const u8,
prefix_lowest_index: u32,
dict_start_index: u32,
dict_index_delta: u32,
dict_end: *const u8,
prefix_lowest: *const u8,
@@ -810,17 +845,22 @@ unsafe fn post_dict_match(
while ptr_le(ip, ilimit) {
let current2 = unsafe { index_from(base, ip) };
let rep_index2 = current2.wrapping_sub(*offset2);
let Some(rep_index2) = attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current2,
0,
*offset2,
) else {
break;
};
let rep_match2 = if rep_index2 < prefix_lowest_index {
dict_base
.wrapping_sub(dict_index_delta as usize)
.wrapping_add(rep_index2 as usize)
dict_base.wrapping_add((rep_index2 - dict_index_delta) as usize)
} else {
base.wrapping_add(rep_index2 as usize)
};
if index_overlap_check(prefix_lowest_index, rep_index2)
&& unsafe { read32(rep_match2) == read32(ip) }
{
if unsafe { read32(rep_match2) == read32(ip) } {
let rep_end2 = if rep_index2 < prefix_lowest_index {
dict_end
} else {
@@ -929,71 +969,78 @@ unsafe fn compress_block_double_fast_dict_match_state(
let mut match_index_small = unsafe { table_get(hash_small, h_small) };
let mut match_long = base.wrapping_add(match_index_long as usize);
let mut matched = base.wrapping_add(match_index_small as usize);
let rep_index = current.wrapping_add(1).wrapping_sub(offset1);
let rep_match = if rep_index < prefix_lowest_index {
dict_base.wrapping_add(rep_index.wrapping_sub(dict_index_delta) as usize)
} else {
base.wrapping_add(rep_index as usize)
};
unsafe {
table_set(hash_long, h_long, current);
table_set(hash_small, h_small, current);
}
if index_overlap_check(prefix_lowest_index, rep_index)
&& unsafe { read32(rep_match) == read32(ip.wrapping_add(1)) }
{
let rep_match_end = if rep_index < prefix_lowest_index {
dict_end
if let Some(rep_index) = attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current,
1,
offset1,
) {
let rep_match = if rep_index < prefix_lowest_index {
dict_base.wrapping_add((rep_index - dict_index_delta) as usize)
} else {
iend
base.wrapping_add(rep_index as usize)
};
let match_length = unsafe {
count_2segments(
ip.wrapping_add(5),
rep_match.wrapping_add(4),
iend,
rep_match_end,
prefix_lowest,
) + 4
};
ip = ip.wrapping_add(1);
unsafe {
store_seq(
seq_store,
ip.offset_from(anchor) as usize,
anchor,
iend,
REPCODE1_TO_OFFBASE,
match_length,
)
};
let (next_ip, next_anchor) = unsafe {
post_dict_match(
hash_long,
hash_small,
base,
dict_base,
prefix_lowest_index,
dict_index_delta,
dict_end,
prefix_lowest,
hash_log,
chain_log,
mls,
seq_store,
&mut offset1,
&mut offset2,
ip,
match_length,
current,
iend,
ilimit,
)
};
ip = next_ip;
anchor = next_anchor;
continue;
if unsafe { read32(rep_match) == read32(ip.wrapping_add(1)) } {
let rep_match_end = if rep_index < prefix_lowest_index {
dict_end
} else {
iend
};
let match_length = unsafe {
count_2segments(
ip.wrapping_add(5),
rep_match.wrapping_add(4),
iend,
rep_match_end,
prefix_lowest,
) + 4
};
ip = ip.wrapping_add(1);
unsafe {
store_seq(
seq_store,
ip.offset_from(anchor) as usize,
anchor,
iend,
REPCODE1_TO_OFFBASE,
match_length,
)
};
let (next_ip, next_anchor) = unsafe {
post_dict_match(
hash_long,
hash_small,
base,
dict_base,
prefix_lowest_index,
dict_start_index,
dict_index_delta,
dict_end,
prefix_lowest,
hash_log,
chain_log,
mls,
seq_store,
&mut offset1,
&mut offset2,
ip,
match_length,
current,
iend,
ilimit,
)
};
ip = next_ip;
anchor = next_anchor;
continue;
}
}
if match_index_long >= prefix_lowest_index && unsafe { read64(match_long) == read64(ip) } {
@@ -1027,6 +1074,7 @@ unsafe fn compress_block_double_fast_dict_match_state(
base,
dict_base,
prefix_lowest_index,
dict_start_index,
dict_index_delta,
dict_end,
prefix_lowest,
@@ -1091,6 +1139,7 @@ unsafe fn compress_block_double_fast_dict_match_state(
base,
dict_base,
prefix_lowest_index,
dict_start_index,
dict_index_delta,
dict_end,
prefix_lowest,
@@ -1278,6 +1327,7 @@ unsafe fn compress_block_double_fast_dict_match_state(
base,
dict_base,
prefix_lowest_index,
dict_start_index,
dict_index_delta,
dict_end,
prefix_lowest,
@@ -1817,3 +1867,83 @@ pub unsafe extern "C" fn ZSTD_rust_compressBlock_doubleFast_extDict(
)
}
}
#[cfg(test)]
mod tests {
use super::{attached_rep_index, index_overlap_check};
#[test]
fn index_overlap_check_preserves_dictionary_and_prefix_boundaries() {
assert!(index_overlap_check(16, 12));
assert!(!index_overlap_check(16, 13));
assert!(index_overlap_check(16, 16));
}
#[test]
fn attached_rep_index_rejects_wrapped_post_match_coordinate() {
// This is the failing shape: 0x1fb - 0x770 wrapped to 0xfffffa8b.
assert_eq!(attached_rep_index(0, 0, 0, 0x1fb, 0, 0x770), None);
}
#[test]
fn attached_rep_index_checks_dictionary_coordinate_boundaries() {
let prefix_lowest_index = 100;
let dict_start_index = 10;
let dict_index_delta = 20;
let current = 120;
// The translated dictionary interval is [30, 100), with four bytes
// available at the upper boundary only through index 96.
assert_eq!(
attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current,
0,
90,
),
Some(30)
);
assert_eq!(
attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current,
0,
91,
),
None
);
assert_eq!(
attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current,
0,
24,
),
Some(96)
);
assert_eq!(
attached_rep_index(
prefix_lowest_index,
dict_start_index,
dict_index_delta,
current,
0,
23,
),
None
);
}
#[test]
fn attached_rep_index_accepts_prefix_boundary_but_not_zero_offset() {
assert_eq!(attached_rep_index(100, 10, 20, 120, 0, 20), Some(100));
assert_eq!(attached_rep_index(100, 10, 20, 120, 1, 21), Some(100));
assert_eq!(attached_rep_index(100, 10, 20, 120, 1, 0), None);
}
}
rust/src/zstd_opt.rs
+27 -5
View File
@@ -363,8 +363,14 @@ fn lowest_match_index(state: &ZSTD_RustOptState, current: u32) -> u32 {
}
#[inline]
fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32) -> bool {
prefix_lowest_index.wrapping_sub(1).wrapping_sub(rep_index) >= 3
fn index_overlap_check(prefix_lowest_index: u32, rep_index: u32, current: u32) -> bool {
if rep_index > current {
return false;
}
rep_index >= prefix_lowest_index
|| rep_index
.checked_add(4)
.is_some_and(|rep_end| rep_end <= prefix_lowest_index)
}
#[inline]
@@ -1006,7 +1012,7 @@ unsafe fn insert_bt_and_get_all_matches(
};
if dict_mode == DICT_EXT
&& rep_offset.wrapping_sub(1) < current.wrapping_sub(window_low)
&& index_overlap_check(dict_limit, rep_index)
&& index_overlap_check(dict_limit, rep_index, current)
&& unsafe { read_min_match(ip, min_match) }
== unsafe { read_min_match(rep_match, min_match) }
{
@@ -1023,7 +1029,7 @@ unsafe fn insert_bt_and_get_all_matches(
if dict_mode == DICT_MATCH_STATE
&& rep_offset.wrapping_sub(1)
< current.wrapping_sub(dms_low_limit.wrapping_add(dms_index_delta))
&& index_overlap_check(dict_limit, rep_index)
&& index_overlap_check(dict_limit, rep_index, current)
&& unsafe { read_min_match(ip, min_match) }
== unsafe { read_min_match(rep_match, min_match) }
{
@@ -1849,9 +1855,25 @@ pub unsafe extern "C" fn ZSTD_rust_compressBlock_btultra2(
#[cfg(test)]
mod tests {
use super::{downscale_stats, refresh_window_projection, ZSTD_RustOptState};
use super::{
downscale_stats, index_overlap_check, refresh_window_projection, ZSTD_RustOptState,
};
use std::ptr;
#[test]
fn index_overlap_check_accepts_exact_non_overlapping_boundary() {
assert!(index_overlap_check(16, 12, 16));
assert!(!index_overlap_check(16, 13, 16));
assert!(index_overlap_check(16, 16, 16));
}
#[test]
fn index_overlap_check_rejects_wrapped_indices() {
assert!(!index_overlap_check(0, u32::MAX - 3, 0x1fb));
assert!(!index_overlap_check(3, u32::MAX, 2));
assert!(index_overlap_check(u32::MAX, u32::MAX, u32::MAX));
}
#[test]
fn ultra2_window_projection_tracks_the_second_pass_window() {
let mut base = 0x2000usize as *const u8;