crypto/openssl: BN_CTX memory grows with every modexp and modinv op
https://bugs.dpdk.org/show_bug.cgi?id=2080
Bug ID: 2080
Summary: crypto/openssl: BN_CTX memory grows with every modexp
and modinv op
Product: DPDK
Version: 26.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: cryptodev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Created attachment 428
--> https://bugs.dpdk.org/attachment.cgi?id=428&action=edit
AI proposed patch
Every modexp and modinv op processed by the crypto_openssl PMD permanently adds two BIGNUMs to the session's BN_CTX. A long-lived session therefore grows the process heap by a fixed amount per op (1124 bytes per 2048-bit modexp, 628 per modinv) until the session is freed. An application that keeps one of these sessions and runs many ops on it will eventually see ops complete with RTE_CRYPTO_OP_STATUS_ERROR once allocation fails inside BN_mod_exp(), or the kernel kills the process if no memory limit is set.
The cause is in process_openssl_modexp_op() and process_openssl_modinv_op() in drivers/crypto/openssl/rte_openssl_pmd.c. Each calls BN_CTX_get() twice on the session context without its own BN_CTX_start()/BN_CTX_end() pair. The only open frame on that context is the one openssl_set_asym_session_parameters() opens at session creation, and openssl_reset_asym_session() closes it at session release, so every op takes two new entries from the pool and none are ever reused. BN_clear() only zeroes the values. The default build is affected, no special input is involved, and ASan/LSan report nothing because the memory is released with the session. The attached reproducer runs 2 × 10000 modexp ops on one session and shows about 11 MB of growth per batch, nearly all of it returned by rte_cryptodev_asym_session_free(). dpdk-test-crypto-perf --optype modex --total-ops 1000000 peaks around 631 MiB RSS for the same reason. The attached patch wraps each op in its own nested BN_CTX_start()/BN_CTX_end() frame, which brings per-op growth to zero. It would carry Fixes: 3e9d6bd447fb and Cc: stable, like the earlier per-op leak fixes in this driver.
This is an ordinary bug, not a security issue: the ops that trigger it are well-formed, nothing in DPDK lets an
outside party drive modexp or modinv ops on a crypto_openssl session, and an application's own ordinary load hits
it just the same.
## Details
Every modexp or modinv op processed by the crypto_openssl PMD takes two BIGNUMs from the session's BN_CTX and
never returns them. The memory is held by the session and grows by a fixed amount per op until the session is freed.
Commit tested: 9a45341f18a1ab47af7123e63d942b90c1b79f38 (26.11-rc1), default build (/src/build), OpenSSL 3.5.5.
A small program creates one RTE_CRYPTO_ASYM_XFORM_MODEX session (2048-bit modulus, exponent 65537), reuses one op
and one result buffer for N successful exponentiations, and reads glibc's mallinfo2().uordblks before the ops,
after them, and after rte_cryptodev_asym_session_free():
| ops on the session | heap growth | returned by freeing the session | |---|---|---| | 1000 | 1 127 120 bytes | 1 100 720 bytes | | 2000 | 2 251 120 bytes | 2 224 720 bytes | | 20000 | 22 483 120 bytes | 22 456 720 bytes |
That is 1124 bytes per op. The same is visible without any custom code: the in-tree perf tool run as below peaks at
about 631 MiB RSS, against about 106 MiB with --total-ops 100000:
```
/src/build/app/dpdk-test-crypto-perf -l 0-1 --no-huge -m 512 --no-pci --vdev
crypto_openssl -- \
--devtype crypto_openssl --ptest throughput --optype modex --total-ops
1000000 --silent
```The ASan build prints nothing for this: the memory stays owned by the session and is released with it, so
LeakSanitizer has nothing to report.
The op handler, drivers/crypto/openssl/rte_openssl_pmd.c:2263:
```c
static int
process_openssl_modexp_op(struct rte_crypto_op *cop,
struct openssl_asym_session *sess)
{
struct rte_crypto_asym_op *op = cop->asym;
BIGNUM *base = BN_CTX_get(sess->u.e.ctx);
BIGNUM *res = BN_CTX_get(sess->u.e.ctx);
…
base = BN_bin2bn((const unsigned char *)op->modex.base.data,
op->modex.base.length, base);
if (BN_mod_exp(res, base, sess->u.e.exp,
sess->u.e.mod, sess->u.e.ctx)) {
…
BN_clear(res);
BN_clear(base);
return 0;
}
```BN_CTX_get() hands out the next unused BIGNUM of the context's pool. A BIGNUM obtained this way becomes reusable
only when BN_CTX_end() closes the frame that the enclosing BN_CTX_start() opened. For sess->u.e.ctx there is
exactly one such frame, and it spans the whole life of the session:
- openssl_set_asym_session_parameters() opens it when the session is created (drivers/crypto/openssl/rte_openssl_pmd_ops.c:1475, and :1506 for modinv)
and takes the modulus and exponent
from it;
- openssl_reset_asym_session() closes it and frees the context when the session is cleared
(rte_openssl_pmd_ops.c:1937, :1943).
Each op in between runs inside that one open frame. openssl_pmd_enqueue_burst() (rte_openssl_pmd.c:4113) calls process_asym_op(), which dispatches to process_openssl_modexp_op() (:3968); the two BN_CTX_get() calls move the
pool forward by two entries, BN_bin2bn() and BN_mod_exp() allocate digit arrays for them, and BN_clear() at
the end only zeroes the values. Nothing rewinds the pool, so the next op gets two further entries and the pool
allocates a new block of BIGNUMs whenever it runs out. The session ends up holding two BIGNUMs and their digits for
every op it has ever processed. The same sequence in a stand-alone OpenSSL program grows by the same 1124 bytes per
iteration, and by 0 once the two BN_CTX_get() calls sit between BN_CTX_start() and BN_CTX_end().
process_openssl_modinv_op() (rte_openssl_pmd.c:2231) is the same code with BN_mod_inverse() onsess->u.m.ctx; it grows by 628 bytes per op with a 2048-bit modulus.
## Impact
Any application that keeps a modexp or modinv session on crypto_openssl and runs many ops on it sees its process
memory grow steadily. A session is bound to a modulus (and exponent), so an application keeps it for as long as that
key or group is in use, and every op on it adds to the process heap for the life of the session: 1124 bytes per
2048-bit modexp, about 950 000 ops per GiB. How fast that adds up depends on the exponent: with exponent 65537 the
test program runs about 40 000 ops/s on one core and holds 214 MiB after 200 000 ops (about 5 seconds); with a
2048-bit exponent it runs about 600 ops/s per core, about 0.5 MiB/s. All ops succeed until memory runs out; then the
allocation fails inside BN_mod_exp() and the op comes back RTE_CRYPTO_OP_STATUS_ERROR (under a 1 GiB data limit,
after about 0.9 to 1.1 million ops), or, without a limit, the kernel kills the process. Freeing and recreating the
session is the only way to get the memory back.
The in-tree users of modexp are dpdk-test-crypto-perf, dpdk-test-eventdev and the unit tests; vhost-crypto
forwards only RSA requests from a guest, and RSA does not take this path. The default build is affected; no
configuration beyond using the OpenSSL PMD for these two op types is needed. The code dates from the introduction of
the asym ops in 3e9d6bd447fb (2018). Earlier per-op leaks in this driver were fixed in b7b06b3fa4 ("crypto/openssl:
fix memory leak in auth processing") and 9d91c3047d ("crypto/openssl: fix memory leaks in asym operations").
## Reproducing
repro builds repro.d/modexp_session.c against the prebuilt default tree (/src/build, no sanitizer) withpkg-config, runs it once with --vdev crypto_openssl, and decides from the heap counters it prints. About
2 seconds.
- repro.d/modexp_session.c is the application, the part an ordinary developer
writes: it configures cryptodev 0
with one queue pair, creates one RTE_CRYPTO_ASYM_XFORM_MODEX session
(modulus 256 bytes of 0xc5, exponent
65537), attaches it to one rte_crypto_op, and enqueues and dequeues that op
for two batches of 10000
modexps, a different base each time. Any op that does not come backRTE_CRYPTO_OP_STATUS_SUCCESS stops it with
an error. It prints glibc's mallinfo2().uordblks before the ops, after each
batch, and after
rte_cryptodev_asym_session_free().
- There is no crafted input: every op is a well-formed modexp. The growth depends only on how many ops the
application runs on the session.
- repro is the setup and the verdict. It prints the application's four heap lines, then the growth in each batch
and how much of the total growth freeing the session gave back. It printsBUG when the second batch still grows
the heap by at least 512 bytes per op (the digits of the two 2048-bit BIGNUMs
that each op takes from the
session's BN_CTX), and freeing the session gives back at least 90 % of the
growth, which ties the memory to the
session.
Expected output on the tested commit:
``` after 0 ops: heap_in_use=255920 after 10000 ops: heap_in_use=11503216 after 20000 ops: heap_in_use=22743216 after session free: heap_in_use=285664 OBSERVED first 10000 ops: heap +11247296 bytes (1125 per op) OBSERVED second 10000 ops: heap +11240000 bytes (1124 per op) OBSERVED freeing the session released 22457552 of the 22487296 bytes BUG ```
With the attached patch applied and drivers/librte_crypto_openssl.so.27.0 rebuilt, the same script shows a one-off
growth of 8032 bytes in the first batch, +0 in the second, and prints OK.
## Suggested fix
The attached patch changes process_openssl_modinv_op() and process_openssl_modexp_op() in drivers/crypto/openssl/rte_openssl_pmd.c so that each op opens its own frame on the session context withBN_CTX_start() before taking its two BIGNUMs and closes it with BN_CTX_end() on both the success path and theBN_CTX_get() failure path. OpenSSL frames nest, so the session's own frame holding the modulus and exponent is
untouched. The patch also replaces the BN_free() calls on the failure path, which act on BIGNUMs the context owns
(harmless in practice, since a BIGNUM just handed out by the context has nothing to free), with BN_CTX_end(),
which also clears the context's too_many flag.
```diff
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -2233,12 +2233,14 @@ process_openssl_modinv_op(struct rte_crypto_op *cop,
struct openssl_asym_session *sess)
{
struct rte_crypto_asym_op *op = cop->asym;
- BIGNUM *base = BN_CTX_get(sess->u.m.ctx);
- BIGNUM *res = BN_CTX_get(sess->u.m.ctx);
+ BIGNUM *base, *res;
+
+ BN_CTX_start(sess->u.m.ctx);
+ base = BN_CTX_get(sess->u.m.ctx);
+ res = BN_CTX_get(sess->u.m.ctx);
if (unlikely(base == NULL || res == NULL)) {
- BN_free(base);
- BN_free(res);
+ BN_CTX_end(sess->u.m.ctx);
cop->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
return -1;
}
@@ -2255,6 +2257,7 @@ process_openssl_modinv_op(struct rte_crypto_op *cop,
BN_clear(res);
BN_clear(base);
+ BN_CTX_end(sess->u.m.ctx);
return 0;
}
@@ -2265,12 +2268,14 @@ process_openssl_modexp_op(struct rte_crypto_op *cop,
struct openssl_asym_session *sess)
{
struct rte_crypto_asym_op *op = cop->asym;
- BIGNUM *base = BN_CTX_get(sess->u.e.ctx);
- BIGNUM *res = BN_CTX_get(sess->u.e.ctx);
+ BIGNUM *base, *res;
+
+ BN_CTX_start(sess->u.e.ctx);
+ base = BN_CTX_get(sess->u.e.ctx);
+ res = BN_CTX_get(sess->u.e.ctx);
if (unlikely(base == NULL || res == NULL)) {
- BN_free(base);
- BN_free(res);
+ BN_CTX_end(sess->u.e.ctx);
cop->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
return -1;
}
@@ -2288,6 +2293,7 @@ process_openssl_modexp_op(struct rte_crypto_op *cop,
BN_clear(res);
BN_clear(base);
+ BN_CTX_end(sess->u.e.ctx);
return 0;
}
```With the patch applied to a clean checkout and the driver rebuilt, the reproducer prints OK, a modinv variant no
longer grows with the op count, and cryptodev_openssl_asym_autotest passes. The patch file is a bare diff; its
commit message should carry:
```
Fixes: 3e9d6bd447fb ("crypto/openssl: add RSA and mod asym operations")
Cc: [email protected]
```Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.