LinuxLists.cc - [PATCH 0/2] add omitted release in qat

2016-09-02 14:38:07

Subject: [PATCH 0/2] add omitted release in qat_common

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

---
drivers/crypto/qat/qat_common/qat_uclo.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
---

2016-09-02 14:38:34

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH 1/2] crypto: qat - introduces a variable to handle error codes

Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.

Signed-off-by: Quentin Lambert <[email protected]>

---
drivers/crypto/qat/qat_common/qat_uclo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
{
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+ int ret;

obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
&obj_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+ ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
- if (!obj_handle->uimage_num)
+ if (!obj_handle->uimage_num) {
+ ret = -EFAULT;
goto out_err;
+ }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
&obj_handle->init_mem_tab);
- if (qat_uclo_set_ae_mode(handle))
+ if (qat_uclo_set_ae_mode(handle)) {
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
+ }
return 0;
out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
out_err:
kfree(obj_handle->uword_buf);
- return -EFAULT;
+ return ret;
}

static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,

2016-09-02 14:38:35

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH 2/2] crypto: qat - fix resource release omissions

This issue was found with Hector.

Signed-off-by: Quentin Lambert <[email protected]>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

2016-09-02 14:43:57

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH 0/2][RESEND] add omitted release in qat_common

2016-09-02 14:44:12

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes

2016-09-02 14:43:59

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH 2/2] crypto: qat - fix resource release omissions

In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.

This issue was found with Hector.

Signed-off-by: Quentin Lambert <[email protected]>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||

2016-09-02 14:47:51

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH v2 0/2] add omitted release in qat_common

The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.

-changes since v1
I failed to send the first version properly and it was missing a proper commit
message, just ignore it.

---
drivers/crypto/qat/qat_common/qat_uclo.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
---

2016-09-02 14:47:52

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes

2016-09-02 14:47:53

by Quentin Lambert

[permalink] [raw]

Subject: [PATCH v2 2/2] crypto: qat - fix resource release omissions

2016-09-06 10:19:04

by Cabiddu, Giovanni

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions

Hi Lambert,

On Fri, Sep 02, 2016 at 04:47:53PM +0200, Quentin Lambert wrote:
> In certain cases qat_uclo_parse_uof_obj used to return with an error code
> before releasing all resources. This patch add a jump to the appropriate label
> ensuring that the resources are properly released before returning.
>
> This issue was found with Hector.
Thanks for the patches. This can be easily fixed by moving the kcalloc after
the compatibility check function. What do you think?

---8<---
Subject: [PATCH] crypto: qat - fix leak on error path

Fix a memory leak in an error path in uc loader.

Signed-off-by: Giovanni Cabiddu <[email protected]>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_uclo.c b/drivers/crypto/qat/qat_common/qat_uclo.c
index 9b961b3..e2454d9 100644
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -967,10 +967,6 @@ static int qat_uclo_parse_uof_obj(struct icp_qat_fw_loader_handle *handle)
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;

- obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
- GFP_KERNEL);
- if (!obj_handle->uword_buf)
- return -ENOMEM;
obj_handle->encap_uof_obj.beg_uof = obj_handle->obj_hdr->file_buff;
obj_handle->encap_uof_obj.obj_hdr = (struct icp_qat_uof_objhdr *)
obj_handle->obj_hdr->file_buff;
@@ -982,6 +978,10 @@ static int qat_uclo_parse_uof_obj(struct icp_qat_fw_loader_handle *handle)
pr_err("QAT: UOF incompatible\n");
return -EINVAL;
}
+ obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
+ GFP_KERNEL);
+ if (!obj_handle->uword_buf)
+ return -ENOMEM;
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
--

2016-09-13 12:40:06

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions

On Tue, Sep 06, 2016 at 11:18:51AM +0100, Giovanni Cabiddu wrote:
>
> ---8<---
> Subject: [PATCH] crypto: qat - fix leak on error path
>
> Fix a memory leak in an error path in uc loader.
>
> Signed-off-by: Giovanni Cabiddu <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-09-13 12:56:26

by Quentin Lambert

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions

On 13/09/2016 14:40, Herbert Xu wrote:
> On Tue, Sep 06, 2016 at 11:18:51AM +0100, Giovanni Cabiddu wrote:
>> ---8<---
>> Subject: [PATCH] crypto: qat - fix leak on error path
>>
>> Fix a memory leak in an error path in uc loader.
>>
>> Signed-off-by: Giovanni Cabiddu <[email protected]>
> Patch applied. Thanks.
Sorry, I completly missed Giovanni's message. Good work!

Quentin