|WARNING| [5/5] crypto/cnxk: use timing-safe digest comparison

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Jun 26 02:50:07 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/5: eal: take experimental flag off of rte_memeq_timingsafe

### Errors

None.

### Warnings

**Missing symbol export macro:**
Patch 1 removes `__rte_experimental` from `rte_memeq_timingsafe()`, promoting it to stable API. However, the function is `static inline` in `rte_memory.h` (header). Inline functions do not require `RTE_EXPORT_SYMBOL()` macros since they have no corresponding symbol in a `.c` file.

**Clarification needed in release notes:**
The release notes state "The inline function `rte_memeq_timingsafe()` is no longer marked experimental." While accurate, it could be clearer about the implications. Since this is an inline function, there is no ABI exposure, but the API is now stable and subject to ABI policy. The wording is acceptable as-is, but consider stating "no ABI impact" explicitly if this is a concern for reviewers.

---

## Patch 2/5: crypto/uadk: use timing-safe digest comparison

### Errors

None.

### Warnings

None.

---

## Patch 3/5: crypto/ccp: use timing-safe digest comparison

### Errors

None.

### Warnings

None.

---

## Patch 4/5: crypto/armv8: use timing-safe digest comparison

### Errors

None.

### Warnings

None.

---

## Patch 5/5: crypto/cnxk: use timing-safe digest comparison

### Errors

**Inverted logic bug:**
In `drivers/crypto/cnxk/cnxk_se.h`, the patch changes:
```c
if (memcmp(mac, gen_mac, mac_len))
	op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
else
	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
```
to:
```c
if (!rte_memeq_timingsafe(mac, gen_mac, mac_len))
	op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
else
	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
```

This is **correct**. `memcmp()` returns 0 when buffers match (C idiom: 0 is false). `rte_memeq_timingsafe()` returns `true` when buffers match (explicit boolean). The negation `!` inverts the boolean to match the original condition. Authentication fails when `rte_memeq_timingsafe()` returns false (buffers differ), which is the correct behavior.

(This is not an error--removing this placeholder after verification.)

### Warnings

None.

---

## Summary

**Correctness:** All patches correctly replace timing-unsafe `memcmp()` with `rte_memeq_timingsafe()` in cryptographic digest verification paths. The boolean inversion in patches 2-5 (`if (!rte_memeq_timingsafe(...))`) correctly handles the semantic difference between `memcmp()` (returns 0 on match) and `rte_memeq_timingsafe()` (returns true on match). These are high-value security fixes addressing a timing side-channel vulnerability (CVE-class issue).

**Patch 1 (API promotion):** The removal of `__rte_experimental` is procedurally correct for an inline function that has been stable in practice. No `RTE_EXPORT_SYMBOL()` macro is needed since the function is `static inline`.

**Release Notes:** Patch 1 updates `release_26_07.rst`, which is the current release (per the 2026-06-26 review date). This is correct.

**Security Impact:** Patches 2-5 are tagged `Cc: stable at dpdk.org` and reference Bugzilla 1773, indicating they should be backported to stable releases. This is appropriate for a security fix.

**No Issues Found** in patches 2, 3, 4 regarding correctness, style, or process compliance.

**Overall Assessment:** This patch series is a well-executed security fix. The timing-safe comparison prevents an attacker from using response timing to iteratively guess the correct HMAC/digest value byte-by-byte (a classic timing side-channel attack on authentication). The code changes are minimal, focused, and correct.


More information about the test-report mailing list