|WARNING| [v11,2/2] app: add Wycheproof validation app

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172701

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-06

# DPDK Patch Review

## ERRORS

**Patch 2/2: Missing sensitive data wiping in `run_ecdsa_verify()`**

Line `app/wycheproof_validation/main.c:1829`:
```c
out:
	rte_crypto_op_free(op);
	if (session != NULL)
		rte_cryptodev_asym_session_free(env.dev_id, session);
	return ret;
```

The `digest` array on the stack holds a message digest derived from user input. When the function returns, this buffer is not wiped. If the digest computation fails, the function may return early without the wipe at line 1713, leaving the partial or complete digest in stack memory.

**Suggested fix:** Add `rte_memzero_explicit(digest, sizeof(digest));` before the `return ret;` at line 1836, and before the early return at line 1713.

---

**Patch 2/2: Missing sensitive data wiping in `run_dsa_verify()`**

Line `app/wycheproof_validation/main.c:1507`:
```c
out:
	rte_crypto_op_free(op);
	if (session != NULL)
		rte_cryptodev_asym_session_free(env.dev_id, session);
	return ret;
```

The `digest` array on the stack holds a message digest. The function may return early on error (e.g., line 1462) or reach the normal exit without wiping this buffer.

**Suggested fix:** Add `rte_memzero_explicit(digest, sizeof(digest));` before the `return ret;` at line 1509, and before the early return at line 1462.

---

**Patch 2/2: Unbounded polling loop on device timeout**

Line `app/wycheproof_validation/main.c:561`:
```c
while (rte_cryptodev_dequeue_burst(env.dev_id, 0, &completed, 1) == 0) {
	if (rte_get_timer_cycles() > deadline)
		return NULL;
	rte_pause();
}
```

This loop repeatedly reads `rte_get_timer_cycles()` on every iteration when the PMD is slow or stuck. `rte_get_timer_cycles()` may invoke a serializing instruction (`rdtsc` + memory fence on some architectures), causing excessive overhead in the wait loop.

**Suggested fix:** Move the deadline check outside the tight loop. Read the timer every N iterations (e.g., every 1000), or use a timeout counter incremented each iteration and check against a precomputed iteration limit.

```c
uint64_t iters = 0;
while (rte_cryptodev_dequeue_burst(dev_id, 0, &completed, 1) == 0) {
	rte_pause();
	if (++iters % 1000 == 0 && rte_get_timer_cycles() > deadline)
		return NULL;
}
```

---

## WARNINGS

**Patch 2/2: Missing release notes for new application**

The patch adds a new application (`app/wycheproof_validation/`) and updates `doc/guides/rel_notes/release_26_11.rst` to document it. However, the release notes entry does not mention the move of the FIPS validation application from `examples/` to `app/` (covered in patch 1/2).

**Suggested fix:** Add a release notes entry in patch 1/2 documenting the FIPS validation move, or expand the existing entry in patch 2/2 to cover both changes.

---

**Patch 1/2: Documentation does not match meson.build change**

Line `app/fips_validation/meson.build:7`:
```python
allow_experimental_apis = true
```

The original meson.build sets `allow_experimental_apis = true`. The patch removes this line. The guide does not document this change or warn that the application must not use experimental API after the move.

**Suggested fix:** If the application still uses experimental API, restore the `allow_experimental_apis` line. If it no longer does, document in the commit message that experimental API usage was removed. If experimental API is needed in the future, the build will fail, so this should be intentional.

---

**Patch 2/2: `rte_malloc()` used for non-DMA, non-shared memory**

Lines where `rte_malloc()` is used for decoded hex buffers, digests, and other control-path allocations:
- `app/wycheproof_validation/main.c:467` (hex decode result)
- `app/wycheproof_validation/main.c:673` (AEAD digest)
- `app/wycheproof_validation/main.c:798` (HMAC digest)
- `app/wycheproof_validation/main.c:912` (GMAC digest)
- `app/wycheproof_validation/main.c:1297` (hash op digest)
- `app/wycheproof_validation/main.c:1641` (ECDH out_x, out_y)

None of these are DMA buffers (the mbuf pool handles packet data) or shared between processes. Using `rte_malloc()` wastes hugepage resources and adds overhead. Standard `malloc()` is preferred for control-path allocations.

**Suggested fix:** Replace `rte_malloc()` / `rte_zmalloc()` with `malloc()` / `calloc()` for these buffers. The exception is the DMA descriptor buffers allocated with `RTE_CACHE_LINE_SIZE` alignment and enqueued to the PMD; those should remain `rte_malloc()`.

---

**Patch 2/2: `rte_free_sensitive()` used on non-secret data**

Line `app/wycheproof_validation/main.c:1885`:
```c
rte_free_sensitive(priv);
```

The `priv` buffer holds the ECDH private key from a test vector. Wycheproof vectors are public test data, not production secrets. However, using `rte_free_sensitive()` here is defensive and documents that private keys should be wiped, so this is acceptable.

**No action required.** This is noted for completeness but is not a problem.

---

**Patch 2/2: Structure member documentation missing**

Lines `app/wycheproof_validation/main.c:64` and `app/wycheproof_validation/main.c:1520`, `app/wycheproof_validation/main.c:1783`:
```c
struct app_env {
	uint8_t dev_id;
	...
};

struct dsa_group {
	uint8_t *p;
	...
};

struct ecdsa_group {
	uint8_t *wx;
	...
};
```

The `app_env`, `dsa_group`, and `ecdsa_group` structures have no Doxygen comments. For internal structures, this is acceptable, but adding brief comments would improve readability for future maintainers.

**Suggested fix:** Add single-line comments above each member or a block comment above the structure explaining its purpose.

---

## INFO

**Patch 1/2: Pre-existing code (not introduced by this patch)**

The patch moves files from `examples/fips_validation/` to `app/fips_validation/` without changing their content. Any issues in the moved code (e.g., error handling, coding style) are pre-existing and not introduced by this patch.

**No action required.**

---

**Patch 2/2: Hardcoded digest buffer size**

Line `app/wycheproof_validation/main.c:1274`:
```c
uint8_t digest[64];
```

The `compute_hash()` function uses a fixed 64-byte buffer for the digest. SHA-512 produces a 64-byte digest, so this is sufficient for all supported hash algorithms (SHA-1 through SHA-512). However, the buffer size is not validated against `digest_len` at runtime.

**Suggested improvement:** Add an assertion or runtime check that `digest_len <= sizeof(digest)` before calling the hash operation, to catch future additions of longer hash algorithms.

---

**Patch 2/2: `memcmp()` used for digest comparison (non-secret data)**

Line `app/wycheproof_validation/main.c:837` and similar:
```c
if (vector.tag_len != 0 &&
    memcmp(digest, vector.tag, vector.tag_len) != 0))
```

This is a test tool comparing computed digests against public test vectors, not production code validating attacker-supplied tags. Timing-safe comparison (`rte_memeq_timingsafe()`) is not required here.

**No action required.**

---

**Patch 2/2: Early allocation failure does not report error details**

Lines `app/wycheproof_validation/main.c:653` and similar:
```c
if (session == NULL)
	goto out;
```

When a session or op allocation fails, the function returns `-ENOMEM` but does not log which allocation failed. For a test tool, this is acceptable; the summary reports the failure count. In production code, logging the specific failure would help debugging.

**No action required.**

---

Review-Result: ERROR