aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthew Smith <mgsmith@netgate.com>2021-12-01 20:02:35 +0000
committerNeale Ranns <neale@graphiant.com>2021-12-04 12:52:29 +0000
commitc14b8cfe601b7b56e946e5f518e82a47e223c721 (patch)
tree73e77279155cbce1ea33ef037f36d8d92e04ab91
parent0d05c0d214ffd326e531bea58f3c971bb9a58252 (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>
-rw-r--r--src/vnet/ipsec/esp.h1
-rw-r--r--src/vnet/ipsec/esp_decrypt.c2
-rw-r--r--src/vnet/ipsec/esp_encrypt.c2
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);
}