Fix data corruption during compression of large chunks #11525

teh-cmc · 2025-10-14T09:56:31Z

There are 3 problems involved:

ArrowMsg::uncompressed_size is defined as an int32. That issue speaks for itself.
The encoding path tracks payload sizes using usize, and then silently cast them to int32 when encoding the Protobuf data, leading to gibberish and crashes during decoding.
lz4_flex has similar casting bugs: it can encode 64bit values but cannot decode them. Once again the as keyword is involved.

This PR fixes 1 and 2. All the changes are backward and forward compatible because:

In protobuf, uint64 is a strict superset of int32: both are unsigned varints on the wire.
There exist no valid RRD files out there with negative values for uncompressed_size.

We can already log much bigger chunks than before, but not too big, otherwise lz4 itself will during decoding:

thread 'decompress' panicked at 'attempt to add with overflow'
lz4_flex-0.11.5/src/block/decompress_safe.rs:39
stack backtrace:
   6: core::panicking::panic_fmt
             at core/src/panicking.rs:75:14
   7: core::panicking::panic_const::panic_const_add_overflow
             at core/src/panicking.rs:175:17
   8: std::sys::backtrace::__rust_begin_short_backtrace
   9: core::ops::function::FnOnce::call_once{{vtable.shim}}
  10: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:1966:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at alloc/src/boxed.rs:1966:9
      std::sys::pal::unix::thread::Thread::new::thread_start
             at std/src/sys/pal/unix/thread.rs:97:17
  11: <unknown>
  12: <unknown>

I'll have a follow-up PR for the lz4_flex issue.

Half of fixing RRD decoder panics on allocation: capacity overflow #11516
- The other half requires patching lz4_flex (on it)

Checks:

rerun with --nb_scans 2000
rerun rrd stats with --nb_scans 2000
⛔ rerun with --nb_scans 3000 -> bug in lz4
⛔ rerun rrd stats with --nb_scans 3000 -> bug in lz4
All dataplatform test suites pass
Can load old RRDs no problem

github-actions · 2025-10-14T09:56:56Z

Web viewer built successfully.

Result	Commit	Link	Manifest
✅	`d84b0f2`	https://rerun.io/viewer/pr/11525	`+nightly` `+main`

^{Note: This comment is updated whenever you push a commit.}

teh-cmc · 2025-10-14T10:26:31Z

crates/store/re_log_encoding/src/protobuf_conversions.rs

            }
            crate::codec::Compression::LZ4 => re_protos::log_msg::v1alpha1::Compression::Lz4 as i32,
        },
-        uncompressed_size: payload.uncompressed_size as i32,


The original bug: Payload::uncompressed_size was a usize that could definitely grow beyond i32::MAX, so that silenced cast leads to disaster.

tests/rust/log_benchmark/src/main.rs

teh-cmc · 2025-10-14T10:42:57Z

@rerun-bot full-check

github-actions · 2025-10-14T10:43:32Z

Started a full build: https://github.com/rerun-io/rerun/actions/runs/18493924895

emilk · 2025-10-14T11:09:55Z

Ouch - maybe we should turn on https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation and https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap

EDIT:

Turn on clippy::cast_possible_wrap #11534

emilk · 2025-10-14T11:11:13Z

crates/store/re_protos/proto/rerun/v1alpha1/log_msg.proto

  Compression compression = 2;

-  int32 uncompressed_size = 3;
+  uint64 uncompressed_size = 3;


is this a breaking change? If so, can we make it a non-breaking change by adding a new field instead?

No, see PR description

emilk · 2025-10-14T11:11:59Z

tests/rust/log_benchmark/src/main.rs


+    // Being able to log fast isn't particularly useful if the data happens to be corrupt at the
+    // other end, so make sure we can encode/decode everything that was logged.
+    if check && let Some(storage) = storage {


Is this run on CI?

No, I don't believe any of this stuff runs on CI. Maybe I'll have a look in a follow-up.

teh-cmc · 2025-10-14T12:07:06Z

merge & pray

There are 3 problems involved: 1. `ArrowMsg::uncompressed_size` is defined as an `int32`. That issue speaks for itself. 2. The encoding path tracks payload sizes using `usize`, and then silently cast them to `int32` when encoding the Protobuf data, leading to gibberish and crashes during decoding. 3. `lz4_flex` has similar casting bugs: it can encode 64bit values but cannot decode them. Once again the `as` keyword is involved. This PR fixes 1 and 2. All the changes are backward and forward compatible because: * In protobuf, `uint64` is a strict superset of `int32`: both are unsigned varints on the wire. * There exist no valid RRD files out there with negative values for `uncompressed_size`. We can already log much bigger chunks than before, but not too big, otherwise `lz4` itself will during decoding: ``` thread 'decompress' panicked at 'attempt to add with overflow' lz4_flex-0.11.5/src/block/decompress_safe.rs:39 stack backtrace: 6: core::panicking::panic_fmt at core/src/panicking.rs:75:14 7: core::panicking::panic_const::panic_const_add_overflow at core/src/panicking.rs:175:17 8: std::sys::backtrace::__rust_begin_short_backtrace 9: core::ops::function::FnOnce::call_once{{vtable.shim}} 10: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once at alloc/src/boxed.rs:1966:9 <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once at alloc/src/boxed.rs:1966:9 std::sys::pal::unix::thread::Thread::new::thread_start at std/src/sys/pal/unix/thread.rs:97:17 11: <unknown> 12: <unknown> ``` I'll have a follow-up PR for the `lz4_flex` issue. --- * Half of fixing #11516 * The other half requires patching `lz4_flex` (on it) --- Checks: * [x] `rerun` with `--nb_scans 2000` * [x] `rerun rrd stats` with `--nb_scans 2000` * [x] ⛔ `rerun` with `--nb_scans 3000` -> bug in lz4 * [x] ⛔ `rerun rrd stats` with `--nb_scans 3000` -> bug in lz4 * [x] All dataplatform test suites pass * [x] Can load old RRDs no problem

This is a follow up to #11525. It fixes the remaining half of #11516. We don't want to merge it as is yet. Let's see if we can get the fix merged upstream first, so we don't have to depend on yet another fork: * PSeitz/lz4_flex#192 --- * Fixes #11516 --- Checks: * [x] `rerun` with `--nb_scans 3000` now works * [x] `rerun rrd stats` with `--nb_scans 3000` now works

teh-cmc added 🪳 bug Something isn't working include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Oct 14, 2025

teh-cmc commented Oct 14, 2025

View reviewed changes

tests/rust/log_benchmark/src/main.rs Show resolved Hide resolved

teh-cmc added 4 commits October 14, 2025 12:41

make sure our benchmarks actually yield valid data

997ca07

add very_large_chunk benchmark, which explodes during decoding

618138f

ArrowMsg::uncompressed size is now a 64bit unsigned varint

06d694d

add --check flag to log benchmarks

d84b0f2

teh-cmc force-pushed the cmc/arrowmsg_32bit_overflow branch from 9393571 to d84b0f2 Compare October 14, 2025 10:42

rerun-io deleted a comment from github-actions bot Oct 14, 2025

teh-cmc marked this pull request as ready for review October 14, 2025 10:42

emilk approved these changes Oct 14, 2025

View reviewed changes

teh-cmc merged commit 19b350c into main Oct 14, 2025
97 of 99 checks passed

teh-cmc deleted the cmc/arrowmsg_32bit_overflow branch October 14, 2025 12:07

teh-cmc mentioned this pull request Oct 14, 2025

LZ4 0.12 (fix decoding of unsually large chunks) #11533

Merged

2 tasks

emilk mentioned this pull request Oct 14, 2025

Turn on clippy::cast_possible_wrap #11534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix data corruption during compression of large chunks #11525

Fix data corruption during compression of large chunks #11525

Uh oh!

teh-cmc commented Oct 14, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 14, 2025 •

edited

Loading

Uh oh!

teh-cmc Oct 14, 2025

Uh oh!

Uh oh!

teh-cmc commented Oct 14, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

emilk commented Oct 14, 2025 •

edited

Loading

Uh oh!

emilk Oct 14, 2025 •

edited

Loading

Uh oh!

teh-cmc Oct 14, 2025

Uh oh!

emilk Oct 14, 2025

Uh oh!

teh-cmc Oct 14, 2025

Uh oh!

teh-cmc commented Oct 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix data corruption during compression of large chunks #11525

Fix data corruption during compression of large chunks #11525

Uh oh!

Conversation

teh-cmc commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teh-cmc Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teh-cmc commented Oct 14, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

emilk commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilk Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teh-cmc Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

emilk Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

teh-cmc Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

teh-cmc commented Oct 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

teh-cmc commented Oct 14, 2025 •

edited

Loading

github-actions bot commented Oct 14, 2025 •

edited

Loading

emilk commented Oct 14, 2025 •

edited

Loading

emilk Oct 14, 2025 •

edited

Loading