diff options
| author |   Matthew Smith <mgsmith@netgate.com> | 2021-12-01 20:02:35 +0000 |
|---|---|---|
| committer |   Neale Ranns <neale@graphiant.com> | 2021-12-04 12:52:29 +0000 |
| commit | c14b8cfe601b7b56e946e5f518e82a47e223c721 (patch) | |
| tree | 73e77279155cbce1ea33ef037f36d8d92e04ab91 /src/vnet | |
| parent | 0d05c0d214ffd326e531bea58f3c971bb9a58252 (diff) | |
ipsec: fix async buffer leak
Type: fix
Fixes: f16e9a5507
If an attempt to submit an async crypto frame fails, the buffers that
were added to the frame are supposed to be dropped. This was not
happening and they are leaking, resulting in buffer exhaustion.
There are two issues:
1. The return value of esp_async_recycle_failed_submit() is used to
figure out how many buffers should be dropped. That function calls
vnet_crypto_async_reset_frame() and then returns f->n_elts. Resetting
the frame sets n_elts to 0. So esp_async_recycle_failed_submit() always
returns 0. It is safe to remove the call to reset the frame because
esp_async_recycle_failed_submit() is called in 2 places and a call to
reset the frame is made immediately afterwards in both cases - so it
is currently unnecessary anyway.
2. An array and an index are passed to esp_async_recycle_failed_submit().
The index should indicate the position in the array where indices of the
buffers contained in the frame should be written. Across multiple calls,
the same index value (n_sync) is passed. This means each call may overwrite
the same entries in the array with the buffer indices in the frame rather
than appending them to the entries which were written earlier. Pass n_noop
as the index instead of n_sync.
Change-Id: I525ab3c466965446f6c116f4c8c5ebb678a66d84
Signed-off-by: Matthew Smith <mgsmith@netgate.com>
Diffstat (limited to 'src/vnet')
| -rw-r--r-- | src/vnet/ipsec/esp.h | 1 | ||||
| -rw-r--r-- | src/vnet/ipsec/esp_decrypt.c | 2 | ||||
| -rw-r--r-- | src/vnet/ipsec/esp_encrypt.c | 2 |
3 files changed, 2 insertions, 3 deletions
diff --git a/src/vnet/ipsec/esp.h b/src/vnet/ipsec/esp.h index d179233df49..8d7e0563a59 100644 --- a/src/vnet/ipsec/esp.h +++ b/src/vnet/ipsec/esp.h @@ -171,7 +171,6 @@ esp_async_recycle_failed_submit (vlib_main_t *vm, vnet_crypto_async_frame_t *f, bi++; index++; } - vnet_crypto_async_reset_frame (f); return (f->n_elts); } diff --git a/src/vnet/ipsec/esp_decrypt.c b/src/vnet/ipsec/esp_decrypt.c index f1e8065b8ff..905b13aa81c 100644 --- a/src/vnet/ipsec/esp_decrypt.c +++ b/src/vnet/ipsec/esp_decrypt.c @@ -1254,7 +1254,7 @@ esp_decrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node, { n_noop += esp_async_recycle_failed_submit ( vm, *async_frame, node, ESP_DECRYPT_ERROR_CRYPTO_ENGINE_ERROR, - n_sync, noop_bi, noop_nexts, ESP_DECRYPT_NEXT_DROP); + n_noop, noop_bi, noop_nexts, ESP_DECRYPT_NEXT_DROP); vnet_crypto_async_reset_frame (*async_frame); vnet_crypto_async_free_frame (vm, *async_frame); } diff --git a/src/vnet/ipsec/esp_encrypt.c b/src/vnet/ipsec/esp_encrypt.c index 8ac0e1a2be7..7fa5ec98c29 100644 --- a/src/vnet/ipsec/esp_encrypt.c +++ b/src/vnet/ipsec/esp_encrypt.c @@ -1067,7 +1067,7 @@ esp_encrypt_inline (vlib_main_t *vm, vlib_node_runtime_t *node, { n_noop += esp_async_recycle_failed_submit ( vm, *async_frame, node, ESP_ENCRYPT_ERROR_CRYPTO_ENGINE_ERROR, - n_sync, noop_bi, noop_nexts, drop_next); + n_noop, noop_bi, noop_nexts, drop_next); vnet_crypto_async_reset_frame (*async_frame); vnet_crypto_async_free_frame (vm, *async_frame); } |