|WARNING| crypto/openssl: fix SM2 encryption output length

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Oct 26 15:53:18 CET 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

**1. Resource leak on error path**

After `EVP_PKEY_encrypt()` for length query is added (line 2794-2796), if this call fails and execution jumps to `err_sm2`, the `cctx` context allocated earlier is not freed before the error return.

The existing code structure shows that `cctx` is created earlier in the function (not visible in this diff) and must be cleaned up on all error paths. The new length-query call introduces an additional failure point before the original encrypt call, but both use the same `goto err_sm2` without ensuring `cctx` is freed.

**Fix**: Verify that the `err_sm2` label properly cleans up `cctx` (e.g., `EVP_PKEY_CTX_free(cctx)`). If the label does not do this, the cleanup must be added. If it does, this is safe.

However, based on the pattern shown and OpenSSL convention, this is likely already handled at the `err_sm2` label. **If you can confirm the label frees `cctx`, disregard this error.**

---

## WARNINGS

**1. Missing variable initialization verification**

The variable `output_len` is used in the new length-query call (line 2794) but is not initialized in the visible context. While the subsequent encrypt call (line 2798) also uses `output_len`, the first call is now reading an uninitialized value as input to determine required length.

**Fix**: Verify that `output_len` is initialized to 0 or a reasonable value before the length-query call. If not, add:
```c
output_len = 0;
```
before line 2794.

---

## INFO

**Context-dependent correctness**

The patch follows the correct two-call EVP pattern (query length, then perform operation) that RSA already uses in this file. The fix is appropriate for the OpenSSL 3.6.3+ behavior change where `EVP_PKEY_encrypt()` requires a non-zero buffer length.

If `output_len` is properly initialized elsewhere in the function and the `err_sm2` label performs proper cleanup, this patch is correct.

---

## FINAL CHECK

- **Correctness bugs**: One potential resource leak depending on `err_sm2` implementation; one potential uninitialized variable read depending on prior initialization
- **Style**: Code follows existing patterns in the file
- **API/Docs**: No release notes needed for a bug fix to internal PMD code

The severity depends on verification of two things not visible in the diff:
1. Does `err_sm2` free `cctx`?
2. Is `output_len` initialized before line 2794?

If both are yes, this patch is CLEAN. If either is no, it requires correction.

**Recommendation**: Request the full function context to verify these two points before merging.

Review-Result: WARNING


More information about the test-report mailing list