You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "masumi-ryugo (via GitHub)" <gi...@apache.org> on 2026/05/01 15:19:17 UTC
[PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
masumi-ryugo opened a new pull request, #9868:
URL: https://github.com/apache/arrow-rs/pull/9868
# Which issue does this PR close?
N/A — found via `cargo-fuzz` libFuzzer harness over
`ParquetMetaDataReader::parse_and_finish` and
`ParquetRecordBatchReaderBuilder::try_new`. Happy to file a tracking
issue first if maintainers prefer.
# Rationale for this change
`parquet::parquet_thrift::read_thrift_vec` reads a Thrift compact-protocol
list header and then calls `Vec::with_capacity(list_ident.size as usize)`
where `list_ident.size` is a 32-bit varint pulled directly from
attacker-controlled bytes. On a malformed input the value can be close to
`i32::MAX`, which after the per-element-size multiplication is a
multi-GB-to-multi-hundred-GB allocation request. That panics in
`alloc::raw_vec::handle_error` with `capacity overflow` (or OOM-kills the
process on smaller-but-still-huge values) before any element is decoded.
Every public sync API that parses parquet bytes funnels through this
function:
- `ParquetMetaDataReader::parse_and_finish`
- `ParquetMetaDataReader::decode_metadata`
- `SerializedFileReader::new`
- `ParquetRecordBatchReaderBuilder::try_new`
- the `async_reader` family (re-exports `decode_metadata` after
prefetching footer bytes)
So any downstream code that hands attacker-controlled bytes to a parquet
reader gets a panic-on-decode DoS. Per `SECURITY.md` this is a bug, not a
vulnerability — there is no information disclosure or RCE path, only
availability — but it is reachable from every metadata entry point and
worth closing.
# What changes are included in this PR?
- Add a default `remaining_bytes() -> Option<usize>` method to
`ThriftCompactInputProtocol`, returning the number of bytes still
available when the protocol is backed by an in-memory buffer.
- Override it on `ThriftSliceInputProtocol` to return `Some(self.buf.len())`.
- In `read_thrift_vec`, clamp the capacity passed to `Vec::with_capacity`
by `remaining_bytes()` (every Thrift element costs at least one wire
byte, so a list whose declared `size` exceeds the remaining input is
malformed by definition — the same clamp the C++ Thrift runtime
applies). Streaming protocols that cannot report a remaining count
fall back to a 1 MiB cap and grow on push.
- Reject negative sizes explicitly. Although the wire format is unsigned,
a hostile encoder can construct a varint that decodes to a negative
`i32`.
- Three regression tests in the existing `parquet_thrift::tests` module:
- `test_read_thrift_vec_huge_size_does_not_panic` exercises the direct
`read_thrift_vec` path with a varint that decodes to ~`i32::MAX`.
- `test_decode_metadata_huge_thrift_list_does_not_panic` runs the
10-byte fuzzer repro through `ParquetMetaDataReader::decode_metadata`
and asserts a clean `Err`.
- `test_read_thrift_vec_negative_size_returns_err` asserts the
explicit negative-size guard.
# Are these changes tested?
Yes — three new unit tests as above. `cargo test -p parquet --release`,
`cargo clippy -p parquet --all-targets -- -D warnings`, and
`cargo fmt --check` are all clean.
(Note: a single pre-existing `should_panic` test
`test_read_non_utf8_binary_as_utf8` aborts during the rust test
harness's panic-formatting machinery on `origin/main` HEAD itself, so
it is excluded with `--skip` for the local run. Unrelated to this PR.)
# Are there any user-facing changes?
Yes — malformed Parquet inputs that previously triggered a `capacity
overflow` panic during metadata decoding now return a `ParquetError`
that callers can handle. No behavior change for well-formed files.
# Reproducer
10 bytes:
```
0x28 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0x51
```
```rust
let bytes: &[u8] = &[0x28, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0x51];
let _ = parquet::file::metadata::ParquetMetaDataReader::decode_metadata(bytes);
```
Before this PR (RUST_BACKTRACE=1):
```
panicked at library/alloc/src/raw_vec/mod.rs:28:5: capacity overflow
3: alloc::raw_vec::handle_error
4: alloc::raw_vec::with_capacity_in
7: alloc::vec::Vec::with_capacity::<SchemaElement>
8: parquet::parquet_thrift::read_thrift_vec (parquet_thrift.rs:690)
9: parquet::file::metadata::thrift::parquet_metadata_from_bytes
(thrift/mod.rs:790)
10: parquet::file::metadata::parser::decode_metadata (parser.rs:233)
13: ParquetMetaDataReader::decode_footer_metadata (reader.rs:783)
14: ParquetMetaDataReader::parse_metadata (reader.rs:585)
17: ParquetMetaDataReader::parse_and_finish (reader.rs:230)
```
After this PR: `Err(ParquetError::EOF("Unexpected EOF"))` (or similar
shape depending on which struct field fails first), no panic.
A 45-byte reproducer that drives the full reader stack (with valid
`PAR1` magic, so it works for `ParquetRecordBatchReaderBuilder::try_new`)
behaves the same way.
# Found via
`cargo-fuzz` libFuzzer harness wrapping
`ParquetMetaDataReader::parse_and_finish` and
`ParquetRecordBatchReaderBuilder::try_new` over `bytes::Bytes`. ~95 unique
crashing inputs (different VLQ sizes, different element types —
`SchemaElement`, `RowGroup`, `ColumnChunk` — different offsets in the
metadata graph) all converged on this single root cause within ~3 minutes
of single-thread fuzzing. One bug, many surface symptoms.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3184460402
##########
parquet/src/parquet_thrift.rs:
##########
@@ -686,8 +704,18 @@ where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
+ // `read_list_begin` rejects sizes above `i32::MAX`; `try_reserve_exact`
+ // catches the remaining attacker-controlled allocation case so a
+ // malformed `size` cannot trigger a `Vec::with_capacity` panic.
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ let len = list_ident.size as usize;
+ let mut res: Vec<T> = Vec::new();
+ if res.try_reserve_exact(len).is_err() {
Review Comment:
I don't understand why this would be reserve_exact
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362329344
run benchmark metadata
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3189142070
##########
parquet/src/parquet_thrift.rs:
##########
@@ -317,7 +329,13 @@ pub(crate) trait ThriftCompactInputProtocol<'a> {
// high bits set high if count and type encoded separately
possible_element_count as i32
} else {
- self.read_vlq()? as _
+ // The list size on the wire is an unsigned varint, but we represent
+ // it as `i32` (matching Java's `int` and the Parquet Thrift schema).
Review Comment:
```suggestion
// The list size on the wire is an unsigned varint, but we represent
// it as `i32` (matching Java's `int` and the Thrift schema).
```
This is actually at the Thrift compact protocol layer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): Prevent negative list sizes in Thrift compact protocol parser [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl merged PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "youichi-uda (via GitHub)" <gi...@apache.org>.
youichi-uda commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4378346120
Thanks @etseidl, @alamb — pushed `65374f8` (force-push) scoping this PR down per your 2026-05-04 review.
**What's still here**
- `read_list_begin`: parse the long-form list size as `i32::try_from(self.read_vlq()?)?`, rejecting any varint above `i32::MAX` at the protocol layer.
- New `ThriftProtocolError::IntegerOverflow` variant + `From<TryFromIntError>` impl, mapped to `general_err!("integer overflow decoding thrift value")`.
- `test_read_list_begin_size_above_i32_max_returns_err` covering the wraparound directly. Verified it fails on base `fd86c75` (returns `Ok(ListIdentifier { size: -2147483648 })`) and passes here.
**What's gone**
- The `read_thrift_vec` switch from `Vec::with_capacity` to `try_reserve_exact` is reverted — answering @alamb's "I don't understand why this would be reserve_exact": agreed it shouldn't, given the page-index regression @etseidl tracked down. `read_thrift_vec` is back to the original `Vec::with_capacity(list_ident.size as usize)`.
- `test_decode_metadata_huge_thrift_list_does_not_panic` removed — that test was a regression guard for the `try_reserve_exact` behavior, so it doesn't belong on a PR that no longer guarantees that.
Net diff vs `fd86c75` is now `parquet/src/parquet_thrift.rs +33/-1` (down from +60/-2).
**Follow-ups, not in this PR**
- Broader `with_capacity` / `reserve` audit on attacker-controlled paths and the OOM-vs-overhead tradeoff continues under #9874.
- The 50-byte ASan OOM repro I posted from a different account on 2026-05-03 is acknowledged and parked — given @etseidl's point that thrift input is compressed (10k-column `SchemaElement` ≈ 96 × 10000 = ~960kB allocation from a ~138kB on-wire schema), an `input_remaining`-style bound would false-positive on valid wide schemas. Any further work on that direction belongs in #9874, not here.
`cargo test -p parquet --release --lib --skip test_read_non_utf8_binary_as_utf8`: 1082 passed. `cargo fmt -p parquet --check`: clean. `cargo clippy -p parquet --lib --no-deps`: clean.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364455219
run benchmark metadata
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "masumi-ryugo (via GitHub)" <gi...@apache.org>.
masumi-ryugo commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362206933
Thanks for the review @etseidl, and thanks for running the metadata bench — happy that the `remaining_bytes()` clamp doesn't show up in the numbers.
I've filed #9874 as the umbrella for both this immediate fix and the broader thrift-parser hardening you mentioned. PR description here is updated to reference it.
Happy to do further fuzzing passes against specific suspected hotspots in `parquet_thrift` if you have particular spots in mind — otherwise I'll do a sweep of remaining `with_capacity`/`reserve` call sites on attacker-controlled paths and either fold those into this PR or file follow-ups under #9874, whichever you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "masumi-ryugo (via GitHub)" <gi...@apache.org>.
masumi-ryugo commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3175797082
##########
parquet/src/parquet_thrift.rs:
##########
@@ -680,14 +698,27 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list `size` is read by [`ThriftCompactInputProtocol::read_list_begin`],
+/// which already rejects values above `i32::MAX`. The up-front allocation
+/// goes through [`Vec::try_reserve_exact`] so that an attacker-supplied
+/// `size` that is below `i32::MAX` but still too large to allocate produces
+/// a clean error instead of panicking inside the allocator.
+///
Review Comment:
Done in d06933b — replaced the doc block with a 3-line safety comment immediately above `read_list_begin`. Diff vs base is now +60/-2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3174295986
##########
parquet/src/parquet_thrift.rs:
##########
@@ -678,16 +697,45 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
}
}
+/// Fallback upper bound for [`Vec::with_capacity`] when the protocol does not
+/// expose a remaining-byte count. Picked conservatively: even the smallest
+/// Thrift element is at least one byte on the wire, and Parquet metadata
+/// payloads larger than this in any single list are well outside the spec.
+const READ_THRIFT_VEC_FALLBACK_CAP: usize = 1 << 20;
+
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list size is taken from the wire as an `i32`. Because that value is
+/// untrusted, the up-front [`Vec::with_capacity`] allocation is bounded by
+/// the number of bytes actually remaining in the input (when the protocol
+/// can report it) — every element on the wire costs at least one byte, so
+/// a list whose `size` exceeds the remaining input is malformed by
+/// definition. Streaming protocols that cannot report a remaining count
+/// fall back to [`READ_THRIFT_VEC_FALLBACK_CAP`].
+///
+/// Negative sizes are rejected explicitly. Although the Thrift spec uses an
+/// unsigned varint, a buggy or hostile encoder can produce a value whose
+/// `i32` interpretation is negative.
+///
/// [list]: https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md#list-and-set
pub(crate) fn read_thrift_vec<'a, T, R>(prot: &mut R) -> Result<Vec<T>>
where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ if list_ident.size < 0 {
+ return Err(general_err!(
+ "Thrift list has negative size {}",
+ list_ident.size
+ ));
+ }
+ let claimed = list_ident.size as usize;
+ let cap = match prot.remaining_bytes() {
+ Some(remaining) => claimed.min(remaining),
+ None => claimed.min(READ_THRIFT_VEC_FALLBACK_CAP),
+ };
+ let mut res = Vec::with_capacity(cap);
Review Comment:
I'm not sure this is the correct fix. This will cap the initial allocation, but then below we continue to read `list_ident.size` elements and push them to the result. If `with_capacity` would fail, the `push` will as well (eventually). Capping `list_ident.size` will leave the parser in an invalid state if we exit decoding the list early (assuming there really are `size` elements present).
Perhaps instead, validation should be moved to `read_list_begin`, which can return an error on an invalid `size`.
Edit: I should have read more. I see below in the test you are expecting the read to eventually hit `EOF`. Also, looking at `read_list_begin`, I think the use of `i32` was a mistake caused by a misreading of the thrift spec. It should be a `u32` instead. But given the limitations imposed by java, I don't think we need to allow anything over 2B anyway.
I think I'm ok with erroring if `size_of::<T>() * size` exceeds `remaining` when the latter is available, but I'm unsure about the capping done here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): Prevent negative list sizes in Thrift compact protocol parser [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4381634344
I took the liberty of updating a comment. I'll merge once CI finishes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379524656
The code now looks unambiguously better to me (checks an error rather than ignores it). If the benchmarks look good I think it is good to go in my opinion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3174295986
##########
parquet/src/parquet_thrift.rs:
##########
@@ -678,16 +697,45 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
}
}
+/// Fallback upper bound for [`Vec::with_capacity`] when the protocol does not
+/// expose a remaining-byte count. Picked conservatively: even the smallest
+/// Thrift element is at least one byte on the wire, and Parquet metadata
+/// payloads larger than this in any single list are well outside the spec.
+const READ_THRIFT_VEC_FALLBACK_CAP: usize = 1 << 20;
+
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list size is taken from the wire as an `i32`. Because that value is
+/// untrusted, the up-front [`Vec::with_capacity`] allocation is bounded by
+/// the number of bytes actually remaining in the input (when the protocol
+/// can report it) — every element on the wire costs at least one byte, so
+/// a list whose `size` exceeds the remaining input is malformed by
+/// definition. Streaming protocols that cannot report a remaining count
+/// fall back to [`READ_THRIFT_VEC_FALLBACK_CAP`].
+///
+/// Negative sizes are rejected explicitly. Although the Thrift spec uses an
+/// unsigned varint, a buggy or hostile encoder can produce a value whose
+/// `i32` interpretation is negative.
+///
/// [list]: https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md#list-and-set
pub(crate) fn read_thrift_vec<'a, T, R>(prot: &mut R) -> Result<Vec<T>>
where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ if list_ident.size < 0 {
+ return Err(general_err!(
+ "Thrift list has negative size {}",
+ list_ident.size
+ ));
+ }
+ let claimed = list_ident.size as usize;
+ let cap = match prot.remaining_bytes() {
+ Some(remaining) => claimed.min(remaining),
+ None => claimed.min(READ_THRIFT_VEC_FALLBACK_CAP),
+ };
+ let mut res = Vec::with_capacity(cap);
Review Comment:
I'm not sure this is the correct fix. This will cap the initial allocation, but then below we continue to read `list_ident.size` elements and push them to the result. If `with_capacity` would fail, the `push` will as well (eventually). Capping `list_ident.size` will leave the parser in an invalid state if we exit decoding the list early (assuming there really are `size` elements present).
Perhaps instead, validation should be moved to `read_list_begin`, which can return an error on an invalid `size`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "masumi-ryugo (via GitHub)" <gi...@apache.org>.
masumi-ryugo commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4378352005
Thanks @etseidl, @alamb — pushed `65374f8` (force-push) scoping this PR down per your 2026-05-04 review.
**What's still here**
- `read_list_begin`: parse the long-form list size as `i32::try_from(self.read_vlq()?)?`, rejecting any varint above `i32::MAX` at the protocol layer.
- New `ThriftProtocolError::IntegerOverflow` variant + `From<TryFromIntError>` impl, mapped to `general_err!("integer overflow decoding thrift value")`.
- `test_read_list_begin_size_above_i32_max_returns_err` covering the wraparound directly. Verified it fails on base `fd86c75` (returns `Ok(ListIdentifier { size: -2147483648 })`) and passes here.
**What's gone**
- The `read_thrift_vec` switch from `Vec::with_capacity` to `try_reserve_exact` is reverted — answering @alamb's "I don't understand why this would be reserve_exact": agreed it shouldn't, given the page-index regression @etseidl tracked down. `read_thrift_vec` is back to the original `Vec::with_capacity(list_ident.size as usize)`.
- `test_decode_metadata_huge_thrift_list_does_not_panic` removed — that test was a regression guard for the `try_reserve_exact` behavior, so it doesn't belong on a PR that no longer guarantees that.
Net diff vs `fd86c75` is now `parquet/src/parquet_thrift.rs +33/-1` (down from +60/-2).
**Follow-ups, not in this PR**
- Broader `with_capacity` / `reserve` audit on attacker-controlled paths and the OOM-vs-overhead tradeoff continues under #9874.
- The 50-byte ASan OOM repro posted from another account on 2026-05-03 is acknowledged and parked — given @etseidl's point that thrift input is compressed (10k-column `SchemaElement` ≈ 96 × 10000 = ~960kB allocation from a ~138kB on-wire schema), an `input_remaining`-style bound would false-positive on valid wide schemas. Any further work on that direction belongs in #9874, not here.
`cargo test -p parquet --release --lib --skip test_read_non_utf8_binary_as_utf8`: 1082 passed. `cargo fmt -p parquet --check`: clean. `cargo clippy -p parquet --lib --no-deps`: clean.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362345836
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362329344)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4362329344-1968-6xtjl 6.12.55+ #1 SMP Sun Feb 1 08:59:41 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing fix/parquet-thrift-bounded-cap (ccc78aa5234ea1755e19280bf7cf57c76f9e0ded) to fd86c75 (merge-base) [diff](https://github.com/apache/arrow-rs/compare/fd86c75c609c664e8d24c780a8a6654729879c0c..ccc78aa5234ea1755e19280bf7cf57c76f9e0ded)
BENCH_NAME=metadata
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench metadata
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3175605685
##########
parquet/src/parquet_thrift.rs:
##########
@@ -1106,4 +1154,55 @@ pub(crate) mod tests {
assert_eq!(header.size, 0);
assert_eq!(header.element_type, ElementType::Byte);
}
+
+ /// Regression test for a Thrift list header that claims a multi-billion
+ /// element size on a few bytes of input.
+ ///
+ /// Prior to capping `Vec::with_capacity` against the remaining input,
+ /// this caused `alloc::raw_vec::handle_error` to panic with
+ /// "capacity overflow" before any element was actually decoded.
+ /// After the fix the capacity is clamped to the remaining buffer and
+ /// the read fails cleanly with EOF when the (still claimed-but-large)
+ /// element loop runs out of input.
+ #[test]
+ fn test_read_thrift_vec_huge_size_does_not_panic() {
Review Comment:
I tried this test with a recent main branch and it passed (returns EOF error).
##########
parquet/src/parquet_thrift.rs:
##########
@@ -1106,4 +1154,55 @@ pub(crate) mod tests {
assert_eq!(header.size, 0);
assert_eq!(header.element_type, ElementType::Byte);
}
+
+ /// Regression test for a Thrift list header that claims a multi-billion
+ /// element size on a few bytes of input.
+ ///
+ /// Prior to capping `Vec::with_capacity` against the remaining input,
+ /// this caused `alloc::raw_vec::handle_error` to panic with
+ /// "capacity overflow" before any element was actually decoded.
+ /// After the fix the capacity is clamped to the remaining buffer and
+ /// the read fails cleanly with EOF when the (still claimed-but-large)
+ /// element loop runs out of input.
+ #[test]
+ fn test_read_thrift_vec_huge_size_does_not_panic() {
+ // Construct a list header that takes the "follow-up varint" branch
+ // (high nibble = 0xF) and supplies a varint that decodes to a
+ // multi-billion `size`. Pre-fix this caused
+ // `Vec::with_capacity(size)` to panic with "capacity overflow"
+ // before any element was decoded.
+ let mut data: Vec<u8> = vec![0xF8]; // element_type=8 (Binary), 0xF=read varint
+ // Pack a varint that decodes to roughly i32::MAX as the size.
+ data.extend_from_slice(&[0xff, 0xff, 0xff, 0xff, 0x07]);
+ let mut prot = ThriftSliceInputProtocol::new(&data);
+ // We don't care whether this returns Ok or Err — only that it does
+ // not panic.
+ let _ = read_thrift_vec::<&[u8], _>(&mut prot);
+ }
+
+ /// As [`test_read_thrift_vec_huge_size_does_not_panic`] but exercising
+ /// the public Parquet metadata entrypoint that the fuzzer reached.
+ #[test]
+ fn test_decode_metadata_huge_thrift_list_does_not_panic() {
Review Comment:
Confirmed that this test and the following one do fail without the fix
##########
parquet/src/parquet_thrift.rs:
##########
@@ -678,16 +697,45 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
}
}
+/// Fallback upper bound for [`Vec::with_capacity`] when the protocol does not
+/// expose a remaining-byte count. Picked conservatively: even the smallest
+/// Thrift element is at least one byte on the wire, and Parquet metadata
+/// payloads larger than this in any single list are well outside the spec.
+const READ_THRIFT_VEC_FALLBACK_CAP: usize = 1 << 20;
+
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list size is taken from the wire as an `i32`. Because that value is
+/// untrusted, the up-front [`Vec::with_capacity`] allocation is bounded by
+/// the number of bytes actually remaining in the input (when the protocol
+/// can report it) — every element on the wire costs at least one byte, so
+/// a list whose `size` exceeds the remaining input is malformed by
+/// definition. Streaming protocols that cannot report a remaining count
+/// fall back to [`READ_THRIFT_VEC_FALLBACK_CAP`].
+///
+/// Negative sizes are rejected explicitly. Although the Thrift spec uses an
+/// unsigned varint, a buggy or hostile encoder can produce a value whose
+/// `i32` interpretation is negative.
+///
/// [list]: https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md#list-and-set
pub(crate) fn read_thrift_vec<'a, T, R>(prot: &mut R) -> Result<Vec<T>>
where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ if list_ident.size < 0 {
+ return Err(general_err!(
+ "Thrift list has negative size {}",
+ list_ident.size
+ ));
+ }
+ let claimed = list_ident.size as usize;
+ let cap = match prot.remaining_bytes() {
+ Some(remaining) => claimed.min(remaining),
+ None => claimed.min(READ_THRIFT_VEC_FALLBACK_CAP),
+ };
+ let mut res = Vec::with_capacity(cap);
Review Comment:
Rather than all this, we can use `Vec::try_reserve_exact` to catch allocation errors.
```rust
let len = list_ident.size as usize;
let mut res = Vec::with_capacity(0);
if let Err(_) = res.try_reserve_exact(len) {
return Err(general_err!("cannot allocate vector"));
}
```
Then we don't need the new `remaining_bytes` function.
##########
parquet/src/parquet_thrift.rs:
##########
@@ -678,16 +697,45 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
}
}
+/// Fallback upper bound for [`Vec::with_capacity`] when the protocol does not
+/// expose a remaining-byte count. Picked conservatively: even the smallest
+/// Thrift element is at least one byte on the wire, and Parquet metadata
+/// payloads larger than this in any single list are well outside the spec.
+const READ_THRIFT_VEC_FALLBACK_CAP: usize = 1 << 20;
+
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list size is taken from the wire as an `i32`. Because that value is
+/// untrusted, the up-front [`Vec::with_capacity`] allocation is bounded by
+/// the number of bytes actually remaining in the input (when the protocol
+/// can report it) — every element on the wire costs at least one byte, so
+/// a list whose `size` exceeds the remaining input is malformed by
+/// definition. Streaming protocols that cannot report a remaining count
+/// fall back to [`READ_THRIFT_VEC_FALLBACK_CAP`].
+///
+/// Negative sizes are rejected explicitly. Although the Thrift spec uses an
+/// unsigned varint, a buggy or hostile encoder can produce a value whose
+/// `i32` interpretation is negative.
+///
/// [list]: https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md#list-and-set
pub(crate) fn read_thrift_vec<'a, T, R>(prot: &mut R) -> Result<Vec<T>>
where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ if list_ident.size < 0 {
Review Comment:
As I've said, I think this should move to `read_list_header`. With a few mods to `ThriftProtocolError`, we can replace
```rust
self.read_vlq()? as _
```
with
```rust
i32::try_from(self.read_vlq()?)?
```
We'd just need conversion from `TryFromIntError` to `ThriftProtocolError`.
I take back what I said about `i32` being wrong. A closer read makes me think a positive integer `<= i32::MAX` is what we want.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3175699088
##########
parquet/src/parquet_thrift.rs:
##########
@@ -680,14 +698,27 @@ impl<'a, R: ThriftCompactInputProtocol<'a>> ReadThrift<'a, R> for &'a [u8] {
/// Read a Thrift encoded [list] from the input protocol object.
///
+/// The list `size` is read by [`ThriftCompactInputProtocol::read_list_begin`],
+/// which already rejects values above `i32::MAX`. The up-front allocation
+/// goes through [`Vec::try_reserve_exact`] so that an attacker-supplied
+/// `size` that is below `i32::MAX` but still too large to allocate produces
+/// a clean error instead of panicking inside the allocator.
+///
Review Comment:
Nit: I think we can do without this in the docs. Perhaps a short comment about safety above the call to `read_list_begin`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360571545
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360553102)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4360553102-1962-gkknb 6.12.55+ #1 SMP Sun Feb 1 08:59:41 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing fix/parquet-thrift-bounded-cap (1d0e7a82f63a6144c631ee14b8b5dc1f05e230af) to fd86c75 (merge-base) [diff](https://github.com/apache/arrow-rs/compare/fd86c75c609c664e8d24c780a8a6654729879c0c..1d0e7a82f63a6144c631ee14b8b5dc1f05e230af)
BENCH_NAME=metadata
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench metadata
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379513385
run benchmark metadata
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360829699
Thanks for the submission @masumi-ryugo. I agree we need to error on invalid input rather than panic. I think we could make a few more improvements to the thrift parser here.
Please do file a separate issue for this. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379544398
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379513385)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4379513385-2027-bjw8r 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing fix/parquet-thrift-bounded-cap (65374f86ea65b3692674e9260810f62cd2c6d53c) to fd86c75 (merge-base) [diff](https://github.com/apache/arrow-rs/compare/fd86c75c609c664e8d24c780a8a6654729879c0c..65374f86ea65b3692674e9260810f62cd2c6d53c)
BENCH_NAME=metadata
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench metadata
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): Prevent negative list sizes in Thrift compact protocol parser [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4381622874
Thanks @masumi-ryugo and @alamb (and @adriangbot 😄)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364672198
I think this is in good shape, but want to investigate the regression. I can't reproduce on my laptop, but will try on my workstation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#discussion_r3184512738
##########
parquet/src/parquet_thrift.rs:
##########
@@ -686,8 +704,18 @@ where
R: ThriftCompactInputProtocol<'a>,
T: ReadThrift<'a, R>,
{
+ // `read_list_begin` rejects sizes above `i32::MAX`; `try_reserve_exact`
+ // catches the remaining attacker-controlled allocation case so a
+ // malformed `size` cannot trigger a `Vec::with_capacity` panic.
let list_ident = prot.read_list_begin()?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ let len = list_ident.size as usize;
+ let mut res: Vec<T> = Vec::new();
+ if res.try_reserve_exact(len).is_err() {
Review Comment:
Because `try_reserve` might overallocate speculatively, but we know the exact length up front.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4373820729
I've tracked the regression down to increased overhead in `try_reserve_exact`. This shows up particularly in the page index bench due to the large number of vector reads that are done in the parsing of the page index structures.
I think for now we should limit the scope of this PR to fixing the wraparound in `read_list_begin`.
The question of how to deal with potential OOM errors should be left to a larger discussion. While an abort is not ideal, that appears to be part of the larger design philosophy of Rust. Given the only way to deal with OOM is to exit anyway, why place an extra burden on every allocation?
As to the suggested alternate fix for this issue, namely erroring if the size of an allocation exceeds the size of the input buffer, that approach too has serious problems. The foremost being that the input stream is highly compressed, meaning the resultant vectors can be much larger. For instance, `size_of::<SchemaElement>()` returns 96 IIRC. But the schema for the wide benchmark (10,000 columns), is on the order of 138kB. `96*10_000` is much larger, and could potentially trigger the proposed OOM detection code for a valid file.
To summarize:
- Let's focus only on detecting wraparound in `read_list_begin`
- Let's move discussion of OOM issues with `with_capacity` to #9874
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364471510
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364455219)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group fix_parquet-thrift-bounded-cap main
----- ------------------------------ ----
decode metadata (wide) with schema 1.00 25.1±0.18ms ? ?/sec 1.03 25.9±0.28ms ? ?/sec
decode metadata (wide) with skip PES 1.00 24.1±0.15ms ? ?/sec 1.07 25.7±0.20ms ? ?/sec
decode metadata (wide) with skip all stats 1.00 27.1±0.27ms ? ?/sec 1.01 27.5±0.28ms ? ?/sec
decode metadata (wide) with skip column stats 1.00 26.4±0.23ms ? ?/sec 1.02 26.9±0.27ms ? ?/sec
decode metadata (wide) with skip size stats 1.00 28.0±0.14ms ? ?/sec 1.06 29.6±0.22ms ? ?/sec
decode metadata (wide) with stats mask 1.00 23.9±0.09ms ? ?/sec 1.07 25.6±0.23ms ? ?/sec
decode metadata with schema 1.00 4.0±0.03µs ? ?/sec 1.02 4.1±0.04µs ? ?/sec
decode metadata with skip PES 1.00 6.7±0.03µs ? ?/sec 1.03 6.9±0.06µs ? ?/sec
decode metadata with skip column stats 1.00 6.7±0.02µs ? ?/sec 1.00 6.7±0.11µs ? ?/sec
decode metadata with stats mask 1.00 6.7±0.03µs ? ?/sec 1.03 6.9±0.07µs ? ?/sec
decode parquet metadata 1.00 7.2±0.02µs ? ?/sec 1.00 7.2±0.05µs ? ?/sec
decode parquet metadata (wide) 1.00 27.3±0.20ms ? ?/sec 1.03 28.1±0.32ms ? ?/sec
decode parquet metadata w/ size stats (wide) 1.00 33.9±0.16ms ? ?/sec 1.03 35.1±0.20ms ? ?/sec
open(default) 1.00 7.6±0.11µs ? ?/sec 1.02 7.8±0.01µs ? ?/sec
open(page index) 1.08 143.1±0.37µs ? ?/sec 1.00 132.4±0.44µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 140.3s |
| CPU sys | 0.7s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 141.3s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364461632
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4364455219)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4364455219-1981-kfm7n 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing fix/parquet-thrift-bounded-cap (d06933b5e3886777e9a871879a3ac89d61fd7721) to fd86c75 (merge-base) [diff](https://github.com/apache/arrow-rs/compare/fd86c75c609c664e8d24c780a8a6654729879c0c..d06933b5e3886777e9a871879a3ac89d61fd7721)
BENCH_NAME=metadata
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench metadata
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "masumi-ryugo (via GitHub)" <gi...@apache.org>.
masumi-ryugo commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362295929
Thanks @etseidl — pushed `ccc78aa` reshaping the fix per all four review comments.
* **`read_list_begin`** (was `parquet_thrift.rs:727` review): the long-form size is now `i32::try_from(self.read_vlq()?)?` instead of `as _`. Varints above `i32::MAX` are rejected at the protocol layer rather than wrapping into a negative `i32`. Added `IntegerOverflow` variant to `ThriftProtocolError` and `From<TryFromIntError>`.
* **`read_thrift_vec`** (was `:738` review): switched to `Vec::try_reserve_exact(size as usize)` per your suggestion. The allocator's `size_of::<T>() * size > isize::MAX` panic is caught and surfaced as `ParquetError`. The `remaining_bytes` trait method and the `ThriftSliceInputProtocol` override are gone — net diff vs base is now `+63/-2` (down from `+100/-1`), and the new abstraction surface is zero.
* **Tests** (`:1168` and `:1186` reviews):
* Dropped `test_read_thrift_vec_huge_size_does_not_panic` — confirmed it passes on main without the fix, so it wasn't a real regression test.
* Kept `test_decode_metadata_huge_thrift_list_does_not_panic` (the 10-byte fuzzer repro through `ParquetMetaDataReader::decode_metadata`).
* Replaced the negative-size test with `test_read_list_begin_size_above_i32_max_returns_err`, which now drives the protocol-layer rejection directly. I verified locally that both tests fail without the fix (capacity overflow + `Ok(ListIdentifier { size: -2147483648 })` respectively) and pass with it.
`cargo test -p parquet --release --lib` (`--skip test_read_non_utf8_binary_as_utf8` per the original PR description note): **1083 passed / 0 failed**. `cargo fmt --check`: clean. `cargo clippy -p parquet --all-targets -- -D warnings`: my changes don't introduce any lints, but I see 17 pre-existing clippy errors on the base `fd86c75` (`arrow-data/src/data.rs`, `parquet/src/arrow/array_reader/byte_array_dictionary.rs`, `parquet/src/arrow/arrow_writer/mod.rs`, `parquet/src/file/page_index/column_index.rs`, `parquet/src/schema/types.rs`, `parquet/src/util/push_buffers.rs`) that I left alone since they're orthogonal to this PR — happy to file/fix separately if useful.
For the broader thrift-parser hardening you mentioned, I'll start sweeping the remaining `with_capacity` / `reserve` call sites on attacker-controlled paths under #9874 unless you'd rather I wait.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379588924
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4379513385)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group fix_parquet-thrift-bounded-cap main
----- ------------------------------ ----
decode metadata (wide) with schema 1.00 27.4±0.86ms ? ?/sec 1.00 27.4±1.06ms ? ?/sec
decode metadata (wide) with skip PES 1.00 26.1±0.33ms ? ?/sec 1.08 28.2±1.30ms ? ?/sec
decode metadata (wide) with skip all stats 1.05 30.1±0.96ms ? ?/sec 1.00 28.6±0.88ms ? ?/sec
decode metadata (wide) with skip column stats 1.00 26.8±0.34ms ? ?/sec 1.09 29.1±1.36ms ? ?/sec
decode metadata (wide) with skip size stats 1.04 31.6±1.32ms ? ?/sec 1.00 30.4±0.33ms ? ?/sec
decode metadata (wide) with stats mask 1.00 27.7±1.15ms ? ?/sec 1.02 28.2±1.00ms ? ?/sec
decode metadata with schema 1.00 4.1±0.01µs ? ?/sec 1.00 4.1±0.01µs ? ?/sec
decode metadata with skip PES 1.00 6.9±0.02µs ? ?/sec 1.00 7.0±0.04µs ? ?/sec
decode metadata with skip column stats 1.00 6.8±0.03µs ? ?/sec 1.01 6.8±0.12µs ? ?/sec
decode metadata with stats mask 1.00 6.9±0.05µs ? ?/sec 1.00 6.9±0.03µs ? ?/sec
decode parquet metadata 1.00 7.2±0.02µs ? ?/sec 1.00 7.2±0.03µs ? ?/sec
decode parquet metadata (wide) 1.03 29.9±1.21ms ? ?/sec 1.00 28.9±0.38ms ? ?/sec
decode parquet metadata w/ size stats (wide) 1.02 37.6±0.58ms ? ?/sec 1.00 36.9±1.13ms ? ?/sec
open(default) 1.00 7.7±0.12µs ? ?/sec 1.00 7.7±0.02µs ? ?/sec
open(page index) 1.00 132.7±0.35µs ? ?/sec 1.00 132.7±0.30µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 150.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 144.2s |
| CPU sys | 0.8s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 143.0s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "youichi-uda (via GitHub)" <gi...@apache.org>.
youichi-uda commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4365657715
Heads-up from the cargo-fuzz harness I'm prototyping for #5332 — this PR's `try_reserve_exact` change reduces but does not fully close the OOM hole on Linux. Concretely:
**Repro** (50 bytes, fits in a single libFuzzer input):
```
00000000 15 20 19 f3 ff ff ff 20 20 20 20 20 20 20 20 20 |. ...... |
00000010 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 | |
00000020 20 20 20 20 20 20 20 20 20 20 2a 00 00 00 50 41 | *...PA|
00000030 52 31 |R1|
```
This is `[footer thrift bytes] + [footer_len=42 LE u32] + "PAR1"`. The thrift bytes encode a list field whose VLQ-encoded size is several billion (`f3 ff ff ff 20 ...`), targeting `FileMetaData.schema: list<SchemaElement>`.
**Result against this PR's `read_thrift_vec`** (built on `pr-9868` HEAD `c36a092`, ASan + libfuzzer):
```
==…== ERROR: libFuzzer: out-of-memory (malloc(6643777440))
…
#11 0x… <alloc::raw_vec::RawVec<…SchemaElement>>::try_reserve_exact
alloc/src/raw_vec/mod.rs:384
#12 0x… parquet::parquet_thrift::read_thrift_vec::<…SchemaElement, ThriftSliceInputProtocol>
parquet/src/parquet_thrift.rs:713
#13 0x… parquet::file::metadata::thrift::parquet_metadata_from_bytes
parquet/src/file/metadata/thrift/mod.rs:790
#14 0x… parquet::file::metadata::parser::decode_metadata
parquet/src/file/metadata/parser.rs:233
#… … decode_footer_metadata / try_parse / parse_and_finish
```
So the new `try_reserve_exact` path is engaged — but on Linux with default overcommit, `try_reserve_exact(N)` still attempts the underlying `malloc(N * sizeof(T))` before returning. ASan's allocator sees the requested size and signals libFuzzer OOM before any `Err(TryReserveError)` can be observed. The `i32::MAX` clamp in `read_list_begin` keeps things off the worst cliff but `i32::MAX * sizeof(SchemaElement)` is still ~6.6 GiB.
**Suggested follow-on (separate PR or grow this one — I'm happy either way)**: bound `len` by the input bytes still available to the protocol before `try_reserve_exact`, mirroring the chunked-read fix in #9869 for `arrow-ipc`. Each thrift element occupies at least 1 byte on the wire, so `min(declared_len, bytes_remaining)` is a sound upper bound. `ThriftSliceInputProtocol` already has `as_slice() -> &[u8]`; promoting a `bytes_remaining(&self) -> usize` to the `ThriftCompactInputProtocol` trait would let `read_thrift_vec` bound the up-front allocation correctly:
```rust
let declared = list_ident.size as usize;
let bound = declared.min(prot.bytes_remaining());
let mut res: Vec<T> = Vec::new();
if res.try_reserve_exact(bound).is_err() {
return Err(general_err!("cannot allocate Thrift list of {declared} elements"));
}
```
That keeps the early-abort intent of this PR but stops the rogue `i32::MAX`-class size from ever reaching the allocator. The legacy hand-rolled `Vec::with_capacity(list_ident.size as usize)` sites in `parquet/src/format.rs` (~22 of them, e.g. lines 5831, 5845 for `Vec<SchemaElement>` / `Vec<RowGroup>`) would need the same treatment if/when they're still on the deserialization path.
Happy to send the trait-method + `read_thrift_vec` patch as a follow-up PR once #5332's harness lands so we have a CI-side regression test for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360601199
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360553102)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group fix_parquet-thrift-bounded-cap main
----- ------------------------------ ----
decode metadata (wide) with schema 1.00 26.0±0.25ms ? ?/sec 1.02 26.5±0.34ms ? ?/sec
decode metadata (wide) with skip PES 1.00 25.0±0.23ms ? ?/sec 1.05 26.2±0.27ms ? ?/sec
decode metadata (wide) with skip all stats 1.00 27.7±0.42ms ? ?/sec 1.01 28.1±0.27ms ? ?/sec
decode metadata (wide) with skip column stats 1.00 27.0±0.24ms ? ?/sec 1.01 27.2±0.24ms ? ?/sec
decode metadata (wide) with skip size stats 1.00 29.1±0.23ms ? ?/sec 1.04 30.3±0.29ms ? ?/sec
decode metadata (wide) with stats mask 1.00 25.0±0.22ms ? ?/sec 1.05 26.1±0.24ms ? ?/sec
decode metadata with schema 1.00 4.0±0.02µs ? ?/sec 1.03 4.1±0.01µs ? ?/sec
decode metadata with skip PES 1.00 6.7±0.02µs ? ?/sec 1.04 7.0±0.03µs ? ?/sec
decode metadata with skip column stats 1.00 6.7±0.03µs ? ?/sec 1.02 6.8±0.02µs ? ?/sec
decode metadata with stats mask 1.00 6.7±0.09µs ? ?/sec 1.04 7.0±0.15µs ? ?/sec
decode parquet metadata 1.00 7.0±0.03µs ? ?/sec 1.03 7.2±0.02µs ? ?/sec
decode parquet metadata (wide) 1.00 27.9±0.17ms ? ?/sec 1.03 28.7±0.33ms ? ?/sec
decode parquet metadata w/ size stats (wide) 1.00 34.9±0.27ms ? ?/sec 1.02 35.8±0.24ms ? ?/sec
open(default) 1.00 7.6±0.14µs ? ?/sec 1.03 7.8±0.12µs ? ?/sec
open(page index) 1.00 132.7±0.31µs ? ?/sec 1.00 132.6±1.53µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 141.9s |
| CPU sys | 0.7s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 140.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 138.5s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362217619
> I've filed #9874 as the umbrella for both this immediate fix and the broader thrift-parser hardening you mentioned. PR description here is updated to reference it.
Thank you 🙏
> Happy to do further fuzzing passes against specific suspected hotspots in `parquet_thrift` if you have particular spots in mind — otherwise I'll do a sweep of remaining `with_capacity`/`reserve` call sites on attacker-controlled paths and either fold those into this PR or file follow-ups under #9874, whichever you prefer.
Let's see how this one plays out, then decide how to proceed. I want to be surgical with these changes so we don't kill performance for the majority of files that are well formed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362376207
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9868#issuecomment-4362329344)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group fix_parquet-thrift-bounded-cap main
----- ------------------------------ ----
decode metadata (wide) with schema 1.00 25.2±0.33ms ? ?/sec 1.04 26.1±0.15ms ? ?/sec
decode metadata (wide) with skip PES 1.00 24.1±0.40ms ? ?/sec 1.08 25.9±0.14ms ? ?/sec
decode metadata (wide) with skip all stats 1.00 27.2±0.31ms ? ?/sec 1.02 27.7±0.28ms ? ?/sec
decode metadata (wide) with skip column stats 1.00 26.7±0.47ms ? ?/sec 1.01 26.8±0.17ms ? ?/sec
decode metadata (wide) with skip size stats 1.00 28.0±0.34ms ? ?/sec 1.07 29.9±0.16ms ? ?/sec
decode metadata (wide) with stats mask 1.00 23.9±0.28ms ? ?/sec 1.08 25.9±0.19ms ? ?/sec
decode metadata with schema 1.00 4.0±0.01µs ? ?/sec 1.02 4.1±0.01µs ? ?/sec
decode metadata with skip PES 1.00 6.8±0.03µs ? ?/sec 1.02 6.9±0.04µs ? ?/sec
decode metadata with skip column stats 1.00 6.8±0.04µs ? ?/sec 1.00 6.8±0.06µs ? ?/sec
decode metadata with stats mask 1.00 6.8±0.04µs ? ?/sec 1.03 6.9±0.03µs ? ?/sec
decode parquet metadata 1.00 7.1±0.04µs ? ?/sec 1.01 7.2±0.04µs ? ?/sec
decode parquet metadata (wide) 1.00 27.3±0.40ms ? ?/sec 1.04 28.4±0.30ms ? ?/sec
decode parquet metadata w/ size stats (wide) 1.00 33.8±0.22ms ? ?/sec 1.05 35.3±0.16ms ? ?/sec
open(default) 1.00 7.7±0.12µs ? ?/sec 1.02 7.8±0.05µs ? ?/sec
open(page index) 1.08 143.0±0.39µs ? ?/sec 1.00 132.6±0.32µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 140.6s |
| CPU sys | 0.7s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 145.0s |
| Peak memory | 4.3 GiB |
| Avg memory | 4.2 GiB |
| CPU user | 141.5s |
| CPU sys | 0.1s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(parquet): bound read_thrift_vec capacity by remaining input [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #9868:
URL: https://github.com/apache/arrow-rs/pull/9868#issuecomment-4360553102
run benchmark metadata
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org