| From c06c76602e03bde24ee69a2022a829127e504202 Mon Sep 17 00:00:00 2001 |
| From: Tom Rix <trix@redhat.com> |
| Date: Mon, 13 Jul 2020 07:06:34 -0700 |
| Subject: crypto: qat - fix double free in qat_uclo_create_batch_init_list |
| |
| 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; |
| } |
| |