|WARNING| [v1, 1/1] crypto/openssl: filter legacy ciphers from capability list

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**1. Resource leak in `openssl_pmd_caps_init()`**
```c
EVP_CIPHER *des_cbc = EVP_CIPHER_fetch(NULL, "DES-CBC", "provider=legacy");
if (des_cbc == NULL)
    continue;
EVP_CIPHER_free(des_cbc);
```
**Issue:** When `des_cbc == NULL` (legacy provider unavailable), the function continues without warning the user. The commit message states "Add a warning when DES algorithms are unavailable" but no warning is emitted. Additionally, the variable `des_cbc` should be checked for NULL before use in the context of the loop.

**Suggested fix:** Add the warning as documented in the commit message:
```c
EVP_CIPHER *des_cbc = EVP_CIPHER_fetch(NULL, "DES-CBC", "provider=legacy");
if (des_cbc == NULL) {
    OPENSSL_LOG(WARNING, "DES ciphers unavailable (legacy provider not loaded)");
    continue;
}
EVP_CIPHER_free(des_cbc);
```

---

## WARNINGS

**1. Missing `RTE_EXPORT_SYMBOL` for new internal function**

The new function `openssl_pmd_caps_init()` is declared in `openssl_pmd_private.h` and defined in `rte_openssl_pmd_ops.c`, making it an internal symbol shared between driver compilation units. It should have an `RTE_EXPORT_INTERNAL_SYMBOL` annotation.

**Suggested fix:** Add before the function definition in `rte_openssl_pmd_ops.c`:
```c
RTE_EXPORT_INTERNAL_SYMBOL(openssl_pmd_caps_init)
void
openssl_pmd_caps_init(void)
```

**2. Potential array bounds issue in `openssl_pmd_info_get()`**

The condition `if (openssl_pmd_caps_active[0].op == RTE_CRYPTO_OP_TYPE_UNDEFINED)` checks whether `openssl_pmd_caps_init()` was called, but this logic is fragile:
- If `openssl_pmd_caps_init()` is never called, `openssl_pmd_caps_active` is zero-initialized by the compiler, so `openssl_pmd_caps_active[0].op` will be 0, which equals `RTE_CRYPTO_OP_TYPE_UNDEFINED`, and the code falls back to `openssl_pmd_capabilities`.
- However, if the first capability in the filtered list happens to be filtered out (unlikely but possible if DES-CBC is the first entry), the check would incorrectly treat the filtered list as uninitialized.

**Suggested fix:** Use a dedicated initialization flag:
```c
static bool openssl_pmd_caps_initialized = false;

void
openssl_pmd_caps_init(void)
{
    /* ... existing code ... */
    openssl_pmd_caps_initialized = true;
}

/* In openssl_pmd_info_get(): */
if (openssl_pmd_caps_initialized)
    dev_info->capabilities = openssl_pmd_caps_active;
else
    dev_info->capabilities = openssl_pmd_capabilities;
```

**3. Inefficient repeated `EVP_CIPHER_fetch()` calls**

The `openssl_pmd_caps_init()` function calls `EVP_CIPHER_fetch()` for every DES cipher capability entry. If there are multiple DES variants in the capability list, this performs the same fetch operation multiple times.

**Suggested fix:** Check once at the start of the function:
```c
void
openssl_pmd_caps_init(void)
{
    EVP_CIPHER *des_cbc;
    bool des_available;
    uint32_t i, j = 0;

    des_cbc = EVP_CIPHER_fetch(NULL, "DES-CBC", "provider=legacy");
    des_available = (des_cbc != NULL);
    if (!des_available)
        OPENSSL_LOG(WARNING, "DES ciphers unavailable (legacy provider not loaded)");
    else
        EVP_CIPHER_free(des_cbc);

    for (i = 0; openssl_pmd_capabilities[i].op != RTE_CRYPTO_OP_TYPE_UNDEFINED; i++) {
        const struct rte_cryptodev_capabilities *cap = &openssl_pmd_capabilities[i];

        if (cap->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
                cap->sym.xform_type == RTE_CRYPTO_SYM_XFORM_CIPHER &&
                (cap->sym.cipher.algo == RTE_CRYPTO_CIPHER_DES_CBC ||
                 cap->sym.cipher.algo == RTE_CRYPTO_CIPHER_DES_DOCSISBPI) &&
                !des_available)
            continue;

        openssl_pmd_caps_active[j++] = *cap;
    }
    openssl_pmd_caps_active[j] = openssl_pmd_capabilities[i];
}
```

---

## INFO

**1. Documentation comment style**

The comment above `openssl_pmd_caps_init()` declaration uses `/** ... */` style typically reserved for Doxygen, but this is an internal function. Consider using the standard multi-line comment style for consistency with other internal functions in the driver:
```c
/*
 * Initialize filtered capability list based on legacy provider availability.
 */
extern void openssl_pmd_caps_init(void);
```

**2. Missing release notes**

This patch changes user-visible behavior (DES ciphers disappear from capabilities when legacy provider is unavailable). This should be documented in the release notes under a "Removed Items" or "Known Issues" section.

---

Review-Result: ERROR