andygrove commented on issue #1116:
URL: https://github.com/apache/arrow-java/issues/1116#issuecomment-6479334591
We hit what looks like the other face of this defect in Apache DataFusion
Comet (apache/datafusion-comet#7305): an executor JVM with Native Memory
Tracking enabled aborted with
```
fatal error: NMT corruption: Block at 0x...: footer canary broken at 0x...
(buffer overflow?)
```
while `ZstdCompressionCodec` was compressing Arrow IPC buffers. The
corrupted block was 512 bytes and held a zstd frame that inflates to 437 bytes,
with no length prefix: the buffer that `AbstractCompressionCodec.compress`
closes when the output turns out larger than the input.
The `dstCapacity` overstatement that #1119 corrected is a real overrun, not
only a semantic nit. zstd guarantees that it never writes past the capacity it
is given, not that it never writes past `compressBound`: while it entropy-codes
the literals of a block that turns out not to compress, it fills the
destination up to `dstCapacity` before falling back to a stored block, so with
`8 + compressBound(n)` as the capacity it can write up to 8 bytes past the
allocation. The allocator's power-of-two rounding hides it unless
`compressBound(n) + 8` lands within 8 bytes below a power of two
(`compressBound(437) + 8 = 509`, allocated as 512), and without NMT the overrun
lands silently in padding or in the next block, which may be why neither of us
could get a minimal reproducer from the original symptom.
A deterministic reproducer for the 18.3.0 code, run with
`-XX:NativeMemoryTracking=detail` (classpath: arrow-compression, arrow-vector,
arrow-memory-core, arrow-memory-unsafe, arrow-format, flatbuffers, jackson,
slf4j, zstd-jni):
```java
import java.util.Random;
import org.apache.arrow.compression.ZstdCompressionCodec;
import org.apache.arrow.memory.*;
public class ZstdCanary {
public static void main(String[] args) {
try (BufferAllocator allocator = new RootAllocator()) {
ZstdCompressionCodec codec = new ZstdCompressionCodec(1);
for (int len = 430; len <= 441; len++) {
for (int seed = 0; seed < 2000; seed++) {
ArrowBuf in = allocator.buffer(len);
in.setBytes(0, protoLike(len, seed));
in.writerIndex(len);
codec.compress(allocator, in).close(); // aborts the JVM on the
free of an overrun block
}
}
System.out.println("no abort");
}
}
// Random bytes with a scatter of repeated values: structured enough for
zstd to attempt
// entropy coding, random enough for the attempt to fail and run to the
end of dstCapacity.
static byte[] protoLike(int len, int seed) {
Random rnd = new Random(seed * 7919L + len);
byte[] out = new byte[len];
rnd.nextBytes(out);
int tags = 8 + rnd.nextInt(24);
for (int i = 0; i < tags; i++) out[rnd.nextInt(len)] = (byte) (0x08 + 8
* rnd.nextInt(4));
return out;
}
}
```
On Arrow 18.3.0 this aborts within the first few hundred inputs at `len=437`
(and at other lengths in the exposed window), with the Java stack
`AbstractCompressionCodec.compress` -> `BufferLedger.release` ->
`Unsafe.freeMemory0`. With `doCompress` changed as in #1119 (`maxSize` as
`dstCapacity`), the same 24000 inputs run clean. The behaviour is the same on
macOS/aarch64 and Linux/x86_64.
Since the fix is a one-argument change and 18.3.0 and 19.0.0 both ship the
overrun, would a patch release carrying #1119 be possible? Until then we are
carrying a subclass with the corrected `doCompress` in Comet.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
