|WARNING| crypto/armv8: validate xform auth key length

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-13

# DPDK Patch Review

## ERRORS

None.

## WARNINGS

### 1. Missing release notes update

**Finding:** The patch adds new validation that will cause `auth_set_prerequisites()` to fail for keys that were previously accepted (though they would have caused buffer overflows). This is a behavior change that applications may observe as newly rejected configurations.

**Why it matters:** The DPDK contributor guidelines require release notes for changes that affect public API behavior or add new error conditions.

**Suggested fix:** Add a release note entry in `doc/guides/rel_notes/release_25_03.rst` (or the appropriate current release notes file) under a "Crypto Drivers" or "armv8 crypto PMD" section:

```rst
* **Updated armv8 crypto PMD.**

  * Added validation of authentication key length in ``auth_set_prerequisites()``.
    The PMD now returns an error if the key length exceeds ``SHA_BLOCK_MAX``
    or if the key data pointer is NULL, matching the behavior of other crypto PMDs.
```

### 2. Error return value should be a negative errno constant

**Finding:** The function returns `-1` on error:
```c
return -1;
```

**Why it matters:** DPDK convention is to return negative errno values (e.g., `-EINVAL`, `-ENOMEM`) for errors rather than the generic `-1`, which aids debugging and allows callers to distinguish error types.

**Suggested fix:**
```c
if (xform->auth.key.data == NULL ||
        xform->auth.key.length > SHA_BLOCK_MAX) {
    ARMV8_CRYPTO_LOG_ERR("Invalid authentication key provided.");
    return -EINVAL;
}
```

## INFO

### 1. Validation order could check NULL before length (pre-existing pattern consideration)

The validation checks both `NULL` pointer and length in a single condition. While functionally correct (due to short-circuit evaluation), checking the `NULL` pointer first in a separate condition would be slightly more defensive and explicit about the two distinct failure modes.

However, the current combined check is acceptable and follows a pattern seen elsewhere in the codebase. This is a minor style preference, not a requirement.

---

**Summary:** The patch correctly addresses a potential buffer overflow by validating the authentication key length and NULL pointer before use. The validation logic is sound. The main issues are the missing release notes (required for user-visible behavior changes) and the use of `-1` instead of `-EINVAL` for the error return (DPDK convention).

Review-Result: WARNING