| Message ID | 20240517005435.2600277-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | efb9f4f19f8e37fde43dfecebc80292d179f56c6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ipv6: sr: fix memleak in seg6_hmac_init_algo | expand |
On Fri, May 17, 2024 at 08:54:35AM +0800, Hangbin Liu wrote: > seg6_hmac_init_algo returns without cleaning up the previous allocations > if one fails, so it's going to leak all that memory and the crypto tfms. > > Update seg6_hmac_exit to only free the memory when allocated, so we can > reuse the code directly. > > Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support") > Reported-by: Sabrina Dubroca <sd@queasysnail.net> > Closes: https://lore.kernel.org/netdev/Zj3bh-gE7eT6V6aH@hog/ > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Thanks, I agree that this was leaking resources, and that after the updates to seg6_hmac_exit included in this patch it can be used to unwind partial initialisation in seg6_hmac_init_algo(). Reviewed-by: Simon Horman <horms@kernel.org>
2024-05-17, 08:54:35 +0800, Hangbin Liu wrote: > seg6_hmac_init_algo returns without cleaning up the previous allocations > if one fails, so it's going to leak all that memory and the crypto tfms. > > Update seg6_hmac_exit to only free the memory when allocated, so we can > reuse the code directly. > > Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support") > Reported-by: Sabrina Dubroca <sd@queasysnail.net> > Closes: https://lore.kernel.org/netdev/Zj3bh-gE7eT6V6aH@hog/ > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Thanks Hangbin.
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Fri, 17 May 2024 08:54:35 +0800 you wrote: > seg6_hmac_init_algo returns without cleaning up the previous allocations > if one fails, so it's going to leak all that memory and the crypto tfms. > > Update seg6_hmac_exit to only free the memory when allocated, so we can > reuse the code directly. > > Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support") > Reported-by: Sabrina Dubroca <sd@queasysnail.net> > Closes: https://lore.kernel.org/netdev/Zj3bh-gE7eT6V6aH@hog/ > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > [...] Here is the summary with links: - [net] ipv6: sr: fix memleak in seg6_hmac_init_algo https://git.kernel.org/netdev/net/c/efb9f4f19f8e You are awesome, thank you!
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c index 861e0366f549..bbf5b84a70fc 100644 --- a/net/ipv6/seg6_hmac.c +++ b/net/ipv6/seg6_hmac.c @@ -356,6 +356,7 @@ static int seg6_hmac_init_algo(void) struct crypto_shash *tfm; struct shash_desc *shash; int i, alg_count, cpu; + int ret = -ENOMEM; alg_count = ARRAY_SIZE(hmac_algos); @@ -366,12 +367,14 @@ static int seg6_hmac_init_algo(void) algo = &hmac_algos[i]; algo->tfms = alloc_percpu(struct crypto_shash *); if (!algo->tfms) - return -ENOMEM; + goto error_out; for_each_possible_cpu(cpu) { tfm = crypto_alloc_shash(algo->name, 0, 0); - if (IS_ERR(tfm)) - return PTR_ERR(tfm); + if (IS_ERR(tfm)) { + ret = PTR_ERR(tfm); + goto error_out; + } p_tfm = per_cpu_ptr(algo->tfms, cpu); *p_tfm = tfm; } @@ -383,18 +386,22 @@ static int seg6_hmac_init_algo(void) algo->shashs = alloc_percpu(struct shash_desc *); if (!algo->shashs) - return -ENOMEM; + goto error_out; for_each_possible_cpu(cpu) { shash = kzalloc_node(shsize, GFP_KERNEL, cpu_to_node(cpu)); if (!shash) - return -ENOMEM; + goto error_out; *per_cpu_ptr(algo->shashs, cpu) = shash; } } return 0; + +error_out: + seg6_hmac_exit(); + return ret; } int __init seg6_hmac_init(void) @@ -412,22 +419,29 @@ int __net_init seg6_hmac_net_init(struct net *net) void seg6_hmac_exit(void) { struct seg6_hmac_algo *algo = NULL; + struct crypto_shash *tfm; + struct shash_desc *shash; int i, alg_count, cpu; alg_count = ARRAY_SIZE(hmac_algos); for (i = 0; i < alg_count; i++) { algo = &hmac_algos[i]; - for_each_possible_cpu(cpu) { - struct crypto_shash *tfm; - struct shash_desc *shash; - shash = *per_cpu_ptr(algo->shashs, cpu); - kfree(shash); - tfm = *per_cpu_ptr(algo->tfms, cpu); - crypto_free_shash(tfm); + if (algo->shashs) { + for_each_possible_cpu(cpu) { + shash = *per_cpu_ptr(algo->shashs, cpu); + kfree(shash); + } + free_percpu(algo->shashs); + } + + if (algo->tfms) { + for_each_possible_cpu(cpu) { + tfm = *per_cpu_ptr(algo->tfms, cpu); + crypto_free_shash(tfm); + } + free_percpu(algo->tfms); } - free_percpu(algo->tfms); - free_percpu(algo->shashs); } } EXPORT_SYMBOL(seg6_hmac_exit);
seg6_hmac_init_algo returns without cleaning up the previous allocations if one fails, so it's going to leak all that memory and the crypto tfms. Update seg6_hmac_exit to only free the memory when allocated, so we can reuse the code directly. Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support") Reported-by: Sabrina Dubroca <sd@queasysnail.net> Closes: https://lore.kernel.org/netdev/Zj3bh-gE7eT6V6aH@hog/ Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- net/ipv6/seg6_hmac.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-)