From: Tom Rix trix@redhat.com
commit c06c76602e03bde24ee69a2022a829127e504202 upstream.
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: stable@vger.kernel.org Signed-off-by: Tom Rix trix@redhat.com Signed-off-by: Herbert Xu herbert@gondor.apana.org.au Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/crypto/qat/qat_common/qat_uclo.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c +++ b/drivers/crypto/qat/qat_common/qat_uclo.c @@ -332,13 +332,18 @@ static int qat_uclo_create_batch_init_li } 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; }