# Patch Analysis: CVE-2024-26582 fix (32b55c5) and the missing 13114dc

## Target & threat model

- **Target:** Linux kernel in-kernel TLS (kTLS) software RX path, `net/tls/tls_sw.c`.
- **CVE:** CVE-2024-26582 — "use-after-free with partial reads and async decrypt".
- **Fix referenced by the ticket:** commit `32b55c5ff9103b8508c1e04bfa5a08c64e7a925f`
  ("net: tls: fix use-after-free with partial reads and async decrypt") — the `free_sgout` fix.
- **kTLS threat model / scope:** kTLS performs record-layer encryption/decryption in the kernel for
  sockets that opt in via `setsockopt(TCP_ULP, "tls")` + `setsockopt(SOL_TLS, TLS_TX/TLS_RX, ...)`.
  The RX decrypt path consumes **network-received ciphertext**: an attacker who can send TLS records
  to a kTLS peer controls those bytes (the trust boundary). Async decryption is enabled for TLS 1.2
  DATA records when the AEAD tfm is async-capable (`CRYPTO_ALG_ASYNC`); TLS 1.3 disables async.
  No `SECURITY.md` carve-out excludes memory-corruption in this path; this is in-scope kernel
  memory safety.

## What the fix `32b55c5` changes

Three edits in `net/tls/tls_sw.c`:

1. **`struct tls_decrypt_ctx`** — add `bool free_sgout;` (records, at submit time, whether
   `tls_decrypt_sg()` allocated the AEAD destination pages).
2. **`tls_decrypt_done()`** (the async AEAD completion callback) — replace the page-free condition:
   ```c
   -	if (sgout != sgin) {            /* buggy pointer-comparison heuristic */
   +	if (dctx->free_sgout) {         /* correct: free only if we allocated them */
   ```
   and drop the now-unused `sgin` local. The body (`for_each_sg(...) put_page(sg_page(sg))`) and
   the trailing `kfree(aead_req)` are unchanged.
3. **`tls_decrypt_sg()`** — set the flag right before submitting the AEAD request:
   ```c
   +	dctx->free_sgout = !!pages;
   ```

`pages` is the count of pages pinned by `tls_setup_from_iter()` (only the ZC `out_iov` sub-case
sets `pages > 0`). So `free_sgout` is `true` iff `tls_decrypt_sg()` actually allocated/pinned the
destination pages.

## Behavior before vs after `32b55c5` (the partial-read UAF)

- **Before (buggy):** `tls_decrypt_done()` freed the destination pages whenever `sgout != sgin`.
  Because `sgout = &dctx->sg[n_sgin]` and `sgin = &dctx->sg[0]` are always distinct arrays, this
  was `true` for **all** sub-cases — including the clear_skb (non-ZC) case where `sgout` points at
  pages that belong to the freshly-allocated `clear_skb` and were **not** pinned by us. The callback
  therefore freed `clear_skb`'s pages; a subsequent partial read caused `process_rx_list()` to
  access them → UAF.
- **After (`32b55c5`):** for clear_skb (`pages == 0`), `free_sgout = false`, so `tls_decrypt_done()`
  does **not** free those pages; `process_rx_list()` is safe. For ZC `out_iov` (`pages > 0`),
  `free_sgout = true` (pages were pinned by us) — freed once by the callback, never by the error
  path in the normal async flow. Correct.

## Assumptions the fix relies on

1. The async completion callback `tls_decrypt_done()` is the **only** place that frees the
   destination pages / `aead_req`-`dctx` block for the async case, **and** it runs exactly once.
2. In the normal async flow (`crypto_aead_decrypt()` → `-EINPROGRESS`), `tls_do_decryption()`
   returns `0` and `tls_decrypt_sg()` returns **without** entering its error cleanup
   (`exit_free_pages` / `exit_free`). Thus the error path and the callback never both free.
3. `dctx->free_sgout` is set before `tls_do_decryption()` in every reachable `tls_decrypt_sg()`
   path (true: the assignment is placed after all `sgout` setup branches and before the AEAD
   submit; `dctx` is freshly `kmalloc`'d per call).

## What the fix does NOT cover (the gap → this variant)

Assumption (2) is **violated by the crypto-backlog (`-EBUSY`) error path**, which `32b55c5` never
touches. `tls_do_decryption()` (v6.6.18, with `32b55c5` and the backlog handling from
`859054147318`):

```c
ret = crypto_aead_decrypt(aead_req);
if (ret == -EBUSY) {                 /* request enqueued to crypto backlog */
    ret = tls_decrypt_async_wait(ctx);   /* SYNCHRONOUSLY wait for backlog to drain */
    ret = ret ?: -EINPROGRESS;           /* if a pending decrypt failed -> -EBADMSG */
}
if (ret == -EINPROGRESS) {
    if (darg->async) return 0;        /* normal async: no error cleanup */
    ret = crypto_wait_req(ret, &ctx->async_wait);
}
darg->async = false;
return ret;
```

When `-EBUSY` occurs **and** a pending async decrypt failed (`ctx->async_wait.err == -EBADMSG`):
- `tls_decrypt_async_wait()` waits until `decrypt_pending` reaches 0; during the wait,
  `tls_decrypt_done()` runs for the backlogged request (and any other pending request) on the
  cryptd workqueue. The callback frees the destination pages (when `free_sgout`) **and**
  `kfree(aead_req)` (the `mem` block).
- `tls_do_decryption()` then returns `-EBADMSG`.
- `tls_decrypt_sg()` does `if (err) goto exit_free_pages;` → frees the destination pages again
  (`put_page` loop over `pages`) and then `exit_free: kfree(mem)` → frees the `aead_req`/`dctx`
  block again.

→ **double-free / slab-use-after-free.** `32b55c5` provides no signal that the callback already
ran, so it cannot prevent the error path from re-freeing. This is exactly the bug commit
`13114dc5543069f7b97991e3b79937b6da05f5b0` ("tls: fix use-after-free on failed backlog decryption",
`Fixes: 859054147318`) fixes.

## Comparison: with `13114dc` applied

`13114dc` adds `bool async_done` to `tls_decrypt_arg.inargs` and:
- in `tls_do_decryption()`, on the `-EBUSY` path sets `darg->async_done = true; darg->async = false;`
  and returns the wait result directly (no `-EINPROGRESS` re-conversion);
- in `tls_decrypt_sg()`, on error after `tls_do_decryption()`, `if (darg->async_done) goto
  exit_free_skb;` — **skipping** both `exit_free_pages` (page free) and `exit_free` (`kfree(mem)`) —
  so the already-freed memory is not freed again; and after the async-hold check,
  `if (unlikely(darg->async_done)) return 0;` (data ready for immediate copy).

We adapted `13114dc` to v6.6.18 (the upstream patch was authored against a slightly different
`tls_do_decryption` base) and confirmed empirically that it eliminates the KASAN UAF under the same
backlog-forcing test (`bundle/logs/vuln_variant/qemu-fixed-ebusy-13114dc.log` — no KASAN).

## Is the fix complete?

**No — `32b55c5` alone is incomplete for the CVE-2024-26582 async-decrypt UAF class.** It closes the
partial-read `process_rx_list()` UAF but leaves the failed-backlog-decryption double-free open on
any kernel that has `32b55c5` and the backlog handling `859054147318` but **not** `13114dc`. Linux
v6.6.18 is exactly such a kernel (verified: `free_sgout` present at `tls_sw.c:66/227/1585`,
`async_done` absent — `grep -c async_done == 0`). The complete fix requires **both** `32b55c5` and
`13114dc`.

## Code paths / inputs the fix does NOT cover (summary)

| Path / input | Reaches `tls_decrypt_done`? | Covered by `32b55c5`? | Notes |
|---|---|---|---|
| `recvmsg` partial read, async, clear_skb (Case A) | yes (normal `-EINPROGRESS`) | **yes** | the original CVE; fixed |
| `recvmsg` full read, async, ZC `out_iov` (Case B) | yes (normal `-EINPROGRESS`) | yes (heuristic happens to be correct) | — |
| `recvmsg` async **+ crypto backlog `-EBUSY` + a failed pending decrypt** | yes (during backlog wait) | **NO** | **this variant/bypass**; needs `13114dc` |
| `splice` (`tls_sw_splice_read`) | no — `darg.async=false` (sync, `crypto_req_done`) | n/a | not a variant |
| `tls_sw_read_sock` / sendfile | no — sync | n/a | not a variant |
| `decrypt_skb()` / `out_sg` (Case C, device fallback) | no — `darg={.zc=true}`, `async=false` (sync) | n/a | not a variant |
| other ciphers (CHACHA20-POLY1305 / AES-256-GCM / AES-128-CCM) | same `tls_decrypt_sg`/`tls_do_decryption`/`tls_decrypt_done` sink | covered by `32b55c5`+`13114dc` (cipher-agnostic) | alternate trigger of same sink, not an independent bypass |