Harden RLE decompression against untrusted indices and offsets - #8953
Conversation
fastlanes 0.6.0 renames `RLE::{encode,decode}` to `unsafe
{encode,decode}_unchecked`, surfacing that the decoder performs
unchecked indexing into the run-values slice.
The encode call site trivially satisfies the contract (all arguments
are 1024-element arrays). The decode path now proves the contract
before entering the unchecked gather:
- `values_idx_offsets` (read from possibly untrusted storage) is
validated once per decompress: offsets must be non-decreasing and
span at most `values.len()`, bounding every per-chunk value slice.
- Chunks referencing zero values are rejected instead of underflowing
the null-position clamp.
- For all-valid chunks, a single SIMD-friendly max-reduction over the
1024 chunk indices verifies they are all below the chunk's value
count before `decode_unchecked`, keeping the bounds check out of the
per-element decode loop.
- The null-clamping path already bounds every index, so it calls
`decode_unchecked` directly.
Adds regression tests that corrupt indices and offsets must fail
decompression with an error rather than read out of bounds.
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L499Tv8CF7Aj7a3cABW9fp
The nullable-indices decode path previously clamped every index into range, which silently remapped a corrupt out-of-bounds index at a *valid* position to the last chunk value. Only null positions may legitimately hold garbage indices (e.g. after further compression of the indices child), so treat the two cases differently: - Materialize the indices validity mask once per decompress, then use a per-chunk SIMD popcount (`count_range`) to classify chunks. - Fully-valid chunks take the existing max-reduction bounds check. - Fully-null chunks skip the gather entirely (all values are masked). - Mixed chunks zero out the null positions via word-at-a-time `for_each_set_index` and then run the same max-reduction check, so an out-of-bounds index at a valid position now fails decompression with an error rather than being clamped. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L499Tv8CF7Aj7a3cABW9fp
b1dfdd4 to
c99326c
Compare
Polar Signals Profiling ResultsLatest Run
Previous Runs (1)
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals Profiling 📖Vortex (geomean): 0.979x ➖ datafusion / vortex-file-compressed / ns (0.979x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: FineWeb NVMe 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.004x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.062x ➖, 1↑ 4↓)
duckdb / vortex-compact / ns (1.011x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.984x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-H SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.995x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.990x ➖, 2↑ 0↓)
duckdb / vortex-file-compressed / ns (1.018x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (1.004x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / duckdb / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Vortex queries 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.015x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.004x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.008x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.054x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.000x ➖, 4↑ 4↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.988x ➖, 9↑ 4↓)
duckdb / vortex-compact / ns (0.997x ➖, 2↑ 3↓)
duckdb / parquet / ns (0.993x ➖, 6↑ 4↓)
duckdb / duckdb / ns (1.002x ➖, 4↑ 5↓)
No file size changes detected. |
Benchmarks: FineWeb S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.024x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.047x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.032x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.052x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.038x ➖, 1↑ 1↓)
|
Benchmarks: Statistical and Population Genetics 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (0.992x ➖, 2↑ 2↓)
duckdb / vortex-compact / ns (0.955x ➖, 3↑ 0↓)
duckdb / parquet / ns (1.001x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Clickbench Sorted on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.036x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.051x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / duckdb / ns (1.007x ➖, 0↑ 0↓)
File Size Changes (201 files changed, +0.0% overall, 90↑ 111↓)
Totals:
|
Benchmarks: TPC-H SF=10 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.007x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.013x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.986x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.990x ➖, 1↑ 0↓)
duckdb / duckdb / ns (0.979x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Random Access 📖Vortex (geomean): 1.005x ➖ vortex / vortex-file-compressed / ns (1.005x ➖, 0↑ 1↓)
vortex / parquet / ns (1.007x ➖, 0↑ 0↓)
vortex / lance / ns (1.003x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.049x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.055x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.008x ➖, 1↑ 1↓)
duckdb / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.965x ➖, 1↑ 0↓)
duckdb / parquet / ns (1.010x ➖, 0↑ 0↓)
|
Benchmarks: Clickbench on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.008x ➖, 0↑ 2↓)
datafusion / parquet / ns (1.004x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.012x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / duckdb / ns (1.001x ➖, 0↑ 0↓)
File Size Changes (1 files changed, -0.0% overall, 0↑ 1↓)
Totals:
|
Benchmarks: Appian on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.995x ➖, 0↑ 0↓)
duckdb / duckdb / ns (0.993x ➖, 0↑ 0↓)
File Size Changes (1 files changed, -0.0% overall, 0↑ 1↓)
Totals:
|
Benchmarks: TPC-H SF=10 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.974x ➖, 1↑ 1↓)
datafusion / vortex-compact / ns (1.072x ➖, 0↑ 2↓)
datafusion / parquet / ns (0.947x ➖, 4↑ 2↓)
duckdb / vortex-file-compressed / ns (0.982x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.006x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.097x ➖, 0↑ 2↓)
|
Benchmarks: Compression 📖vortex / vortex-file-compressed / ns (0.981x ➖, 1↑ 0↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.988x ➖, 1↑ 0↓)
vortex / parquet / ns (0.999x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
|
Merging this PR will improve performance by 11.52%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
Benchmarks: String Encoding 📖vortex / vortex-file-compressed / ms (1.002x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Rationale for this change
The RLE decompression code didn't carefully validate indices.
This PR fixes that.