From c06c76602e03bde24ee69a2022a829127e504202 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Mon, 13 Jul 2020 07:06:34 -0700 Subject: [PATCH] crypto: qat - fix double free in qat_uclo_create_batch_init_list clang static analysis flags this error qat_uclo.c:297:3: warning: Attempt to free released memory [unix.Malloc] kfree(*init_tab_base); ^~~~~~~~~~~~~~~~~~~~~ When input *init_tab_base is null, the function allocates memory for the head of the list. When there is problem allocating other list elements the list is unwound and freed. Then a check is made if the list head was allocated and is also freed. Keeping track of the what may need to be freed is the variable 'tail_old'. The unwinding/freeing block is while (tail_old) { mem_init = tail_old->next; kfree(tail_old); tail_old = mem_init; } The problem is that the first element of tail_old is also what was allocated for the list head init_header = kzalloc(sizeof(*init_header), GFP_KERNEL); ... *init_tab_base = init_header; flag = 1; } tail_old = init_header; So *init_tab_base/init_header are freed twice. There is another problem. When the input *init_tab_base is non null the tail_old is calculated by traveling down the list to first non null entry. tail_old = init_header; while (tail_old->next) tail_old = tail_old->next; When the unwinding free happens, the last entry of the input list will be freed. So the freeing needs a general changed. If locally allocated the first element of tail_old is freed, else it is skipped. As a bit of cleanup, reset *init_tab_base if it came in as null. Fixes: b4b7e67c917f ("crypto: qat - Intel(R) QAT ucode part of fw loader") Cc: Signed-off-by: Tom Rix Signed-off-by: Herbert Xu --- drivers/crypto/qat/qat_common/qat_uclo.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/qat/qat_common/qat_uclo.c b/drivers/crypto/qat/qat_common/qat_uclo.c index 4cc1f436b075..bff759e2f811 100644 --- a/drivers/crypto/qat/qat_common/qat_uclo.c +++ b/drivers/crypto/qat/qat_common/qat_uclo.c @@ -288,13 +288,18 @@ static int qat_uclo_create_batch_init_list(struct icp_qat_fw_loader_handle } return 0; out_err: + /* Do not free the list head unless we allocated it. */ + tail_old = tail_old->next; + if (flag) { + kfree(*init_tab_base); + *init_tab_base = NULL; + } + while (tail_old) { mem_init = tail_old->next; kfree(tail_old); tail_old = mem_init; } - if (flag) - kfree(*init_tab_base); return -ENOMEM; }