[
https://issues.apache.org/jira/browse/SPARK-58777?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated SPARK-58777:
-----------------------------------
Labels: pull-request-available (was: )
> Validate the UnsafeRow backing memory before computing the shuffle row-based
> checksum
> -------------------------------------------------------------------------------------
>
> Key: SPARK-58777
> URL: https://issues.apache.org/jira/browse/SPARK-58777
> Project: Spark
> Issue Type: Bug
> Components: SQL
> Affects Versions: 4.3.0
> Reporter: Mark Jarvin
> Priority: Major
> Labels: pull-request-available
>
> The order-independent shuffle checksum
> (`spark.sql.shuffle.orderIndependentChecksum.enabled`) computes a per-row
> checksum in `UnsafeRowChecksum` by hashing the row's raw backing memory:
> ```
> XXH64.hashUnsafeBytes(row.getBaseObject, row.getBaseOffset,
> row.getSizeInBytes, 0)
> ```
> XXH64 reads 8-byte words straight from (`baseObject`, `baseOffset`) via
> `Platform.getLong`, with no validation of the `UnsafeRow` beforehand. If a
> row reaches the checksum with malformed backing memory (for example a `null`
> off-heap `baseObject` whose `baseOffset` points into the first, unmapped
> memory page, or a negative `sizeInBytes`) the hash dereferences invalid
> memory. The result is an opaque SIGSEGV/JVM crash (or an out-of-bounds read)
> with a stack that points at the checksum, far from whatever produced the
> malformed row, rather than a diagnosable error. Because the checksum can be
> enabled by default, an upstream defect that yields such a row surfaces as a
> hard-to-attribute native crash.
> I propose we add a cheap validation step in `RowBasedChecksum.update` that
> runs before the row is dereferenced. `UnsafeRowChecksum` checks the row's
> `(baseObject, baseOffset, sizeInBytes)` for the cases a validly-constructed
> `UnsafeRow` can never exhibit:
> - a negative `sizeInBytes`;
> - a null `baseObject` whose `baseOffset` lies in the first memory page;
> - an out-of-bounds on-heap (`byte[]`) offset.
> Any well-formed row is accepted (including `long[]`-backed rows), so the
> check never rejects a valid row and a corrupt-but-large size is deliberately
> not treated as invalid. When a row is flagged, log the row's context
> (partition, the row's ordinal within the partition, and its
> `baseObject`/`baseOffset`/`sizeInBytes`) and handle it without performing the
> unchecked read, either failing the task with a descriptive error, or
> disabling that partition's checksum so the query proceeds (consistent with
> the existing NonFatal handling in `RowBasedChecksum.update`, which already
> disables the checksum on a computation error). The exact behavior can be made
> configurable.
> This is defensive hardening of the checksum reader. It does not change
> checksum values or results for valid data. Its value is diagnosability and
> robustness: an upstream defect that produces a malformed row is turned into a
> clear, logged, attributable failure that identifies where the bad row was
> observed, instead of an unchecked native dereference that crashes the JVM at
> a site unrelated to the producer.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
