intel / intel/QATzip

Memory corruption when a session requests a larger `hw_buff_sz` than the first session in the process

Open
#151 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
169
Forks
59
PR merge metrics
No merged PRs in 30d

Description

## Symptom

A process that compresses once at the default `hw_buff_sz` (64 KiB) and later sets up a session with a larger, in-spec `hw_buff_sz` (up to `QZ_HW_BUFF_MAX_SZ` = 512 KiB) dies with SIGSEGV **after `main` returns**, inside the library's own ELF destructor: `exitFunc` → `cleanUpInstMem` → `qzFree` → `free`, on a pointer that is no longer a pointer. The faulting address is literally the caller's payload: the reproducer fills its input with a recognizable `i % 256` pattern, and the pointer being freed is a run of bytes from that pattern.

Both `qzSetupSessionDeflateExt()` and both `qzCompress()` calls return `QZ_OK` first. Nothing reports an error at the point of damage.

Reproducer attached: `repro_hw_buff_latch.c`.

```
$ gcc -O0 -g -Wall -Wextra -o repro_hw_buff_latch repro_hw_buff_latch.c -lqatzip -lz
$ ./repro_hw_buff_latch ; echo "exit=$?"
SMALL_FIRST=1 (expect SIGSEGV at exit)

session A: latches the process-wide buffers
hw_buff_sz=65536 payload=1024 -> 330 bytes, rc=QZ_OK
session A torn down and closed

session B: requests a LARGER hw_buff_sz than the latch
hw_buff_sz=524288 payload=524288 -> 2390 bytes, rc=QZ_OK
session B torn down and closed

returning from main; the library destructor runs after this
Segmentation fault (core dumped)
exit=139

$ SMALL_FIRST=0 ./repro_hw_buff_latch ; echo "exit=$?" # control: large first
...
exit=0
```

**Judge the reproducer by exit status, not by whether it prints `done`** — the heap is already corrupt while both compressions report success.

## Root cause

Everything below — line numbers, the build under test, and the SIGSEGV

**1. The pinned instance buffers are process-wide and sized from the first session to compress.** `getInstMem()` takes the size straight from the session params and allocates each source buffer with it:

```c
/* src/qatzip.c:964 */
/* WARN: this will mean the first sess will setup down the inst
* buffer size, if it's very small, then we can't make then
* any larger in the whole process. please refer test mode 17
*/
src_sz = params->hw_buff_sz;
```
```c
/* src/qatzip.c:1062 */
g_process.qz_inst[i].src_buffers[j]->pBuffers->pData = (Cpa8U *)
qzMalloc(src_sz, QZ_AUTO_SELECT_NUMA_NODE, PINNED_MEM);
```

and then latches at `src/qatzip.c:1111`:

```c
g_process.qz_inst[i].mem_setup = 1;
```

`src_count` / `dest_count` are latched from the same value too (`src/qatzip.c:1017-1021`: `NUM_BUFF_8K` when `hw_buff_sz <= 8K`, else `NUM_BUFF`) — so a fix has **two** latched quantities to consider, not one.

**2. Nothing releases those buffers short of process exit.** `qzClose()` (`src/qatzip.c:2777`) is a no-op. `qzTeardownSession()` (`:2702`) frees only
per-session state and never touches `g_process.qz_inst[i]`. `cleanUpInstMem()` (`:849`) is the only code that clears `mem_setup` after
allocation (`:932`; the other assignment at `:786` is initial zeroing during instance discovery in `qzInit`), and outside the two allocation-failure macros it is reachable only from `exitFunc()` (`:514`), the destructor. `qzUpdateCpaSession()` reallocates `cpaSess` alone. So on any successful run the buffers survive every teardown at the size the first compress chose.

**3. A later, larger `hw_buff_sz` is accepted anyway.** `QZ_HW_BUFF_MAX_SZ` is `512*1024` (`include/qatzip.h:584`) and `qzCheckParams*` validates the request (`src/qatzip_utils.c:453`, `:506`). The value lands in the session params, and `doCompressIn()` chunks against the *session's* value with no reference to what was allocated:

```c
/* src/qatzip.c:1523*/
hw_buff_sz = qz_sess->sess_params.hw_buff_sz; /* 512 KiB */
...
src_send_sz = (remaining < hw_buff_sz) ? remaining : hw_buff_sz;
```
**4. The copy into the undersized buffer is unbounded.** `compBufferSetup()` passes two *source* lengths where `QZ_MEMCPY` expects a destination capacity and a source length:

```c
/* src/qatzip_utils.c:1134*/
QZ_MEMCPY(g_process.qz_inst[i].src_buffers[j]->pBuffers->pData,
src_ptr,
src_send_sz, /* <- a source length, in the dest_sz position */
src_remaining);
```
```c
/* include/qatzip.h:377*/
#define QZ_MEMCPY(dest, src, dest_sz, src_sz) \
memcpy((void *)(dest), (void *) (src), (size_t)MIN(dest_sz, src_sz))
```

`MIN(src_send_sz, src_remaining)` is `src_send_sz` whenever a full block is available, so the `MIN` bounds nothing against the real 64 KiB capacity and the copy overruns by up to 448 KiB. There is no field anywhere in `QzInstance_T` (`src/qatzip_internal.h:173-196`) recording the size the buffers were allocated with, so no existing code *could* perform that check.

What the overrun lands on is not merely allocator metadata but the neighbouring buffer descriptors: with `src_count = 32`, the copy past `src_buffers[0]`'s data buffer overwrites `src_buffers[1]`'s `pPrivateMetaData` and `pBuffers` pointers with payload bytes. `cleanUpInstMem()` then walks those descriptors at exit and `free()`s pointers assembled from caller data — which is the crash in the symptom above.

Note this is USDM memory (`qzMalloc` → `qaeMemAllocNUMA`), not the glibc heap, so **ASAN does not help**: there are no redzones around these buffers, an ASAN build fails at the same destructor `free()` with no extra detail, and the overflowing write itself is never observed.

## Why a caller cannot work around this

- The value is documented, in range, and validated — the library accepts it and then overflows.
- No API releases the instance buffers, so a caller cannot reset the latch.
- No API exposes the latched size, so a caller cannot even *query* what is safe before setting up a session.
- The trigger threshold is one 1 KB compress at the default size. In a shared runtime (our case: a JVM where unrelated components each build their own session) which session compresses first is not under any one caller's control.

The in-tree `WARN` at `qatzip.c:964` shows the constraint is already known - the report is that it is documented but unenforced, and that violating it corrupts memory rather than failing.

## One caveat on silently clamping

One possible fix that looks locally attractive and has a consequence worth stating up front, because we hit it: making the oversized request *silently* fall back to the latched size.

That is appealing since it needs no API change and no reallocation. The problem is that `hw_buff_sz` is not only an allocation size — it is the compression blocking factor. The hardware terminates a **complete** deflate/zlib stream at every `hw_buff_sz` boundary, so quietly reducing the effective value changes the *format* of the output, not just its performance:

A caller that asked for 512 KiB and got 64 KiB blocking receives a sequence of concatenated streams rather than the single stream it expected and could have consequences. If clamping is the direction, the effective value would need to be observable and documented so a caller can tell that its requested framing was not honored.

Contributor guide

Open the contributing guide

Research direction

Start with the reproducer repro_hw_buff_latch.c and test mode 17, then inspect getInstMem and cleanUpInstMem in src/qatzip.c, compBufferSetup in src/qatzip_utils.c, and the session-size handling in doCompressIn. Confirm the first-small/later-large sequence and large-first control, then define behavior that prevents the overwrite and leaves both runs exiting cleanly without changing framing silently.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.