From: Tom Rix trix@redhat.com
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")
Signed-off-by: Tom Rix trix@redhat.com --- 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; }
On Mon, Jul 13, 2020 at 07:06:34AM -0700, trix@redhat.com wrote:
From: Tom Rix trix@redhat.com
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")
Signed-off-by: Tom Rix trix@redhat.com
drivers/crypto/qat/qat_common/qat_uclo.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
<formletter>
This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
</formletter>
On Mon, Jul 13, 2020 at 07:06:34AM -0700, trix@redhat.com wrote:
From: Tom Rix trix@redhat.com
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")
Signed-off-by: Tom Rix trix@redhat.com
drivers/crypto/qat/qat_common/qat_uclo.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Patch applied. Thanks.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Herbert Xu -
trix＠redhat.com