* [PATCH 0/2][RESEND] add omitted release in qat_common
@ 2016-09-02 14:43 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
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(-)
---
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes
2016-09-02 14:43 ` Quentin Lambert
@ 2016-09-02 14:43 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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,
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes
@ 2016-09-02 14:43 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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,
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2] crypto: qat - fix resource release omissions
2016-09-02 14:43 ` Quentin Lambert
@ 2016-09-02 14:43 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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 ||
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 2/2] crypto: qat - fix resource release omissions
@ 2016-09-02 14:43 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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 ||
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/2] add omitted release in qat_common
2016-09-02 14:43 ` Quentin Lambert
@ 2016-09-02 14:47 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
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(-)
---
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/2] add omitted release in qat_common
@ 2016-09-02 14:47 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
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(-)
---
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes
2016-09-02 14:47 ` Quentin Lambert
@ 2016-09-02 14:47 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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,
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes
@ 2016-09-02 14:47 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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,
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 2/2] crypto: qat - fix resource release omissions
2016-09-02 14:47 ` Quentin Lambert
@ 2016-09-02 14:47 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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 ||
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v2 2/2] crypto: qat - fix resource release omissions
@ 2016-09-02 14:47 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
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 <lambert.quentin@gmail.com>
---
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 ||
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
2016-09-02 14:47 ` Quentin Lambert
@ 2016-09-06 10:18 ` Giovanni Cabiddu
-1 siblings, 0 replies; 24+ messages in thread
From: Giovanni Cabiddu @ 2016-09-06 10:18 UTC (permalink / raw)
To: Quentin Lambert
Cc: Salvatore Benedetto, Herbert Xu, David S. Miller, qat-linux,
linux-crypto, linux-kernel, kernel-janitors, giovanni.cabiddu,
giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
---
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,
--
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
@ 2016-09-06 10:18 ` Giovanni Cabiddu
0 siblings, 0 replies; 24+ messages in thread
From: Giovanni Cabiddu @ 2016-09-06 10:18 UTC (permalink / raw)
To: Quentin Lambert
Cc: Salvatore Benedetto, Herbert Xu, David S. Miller, qat-linux,
linux-crypto, linux-kernel, kernel-janitors, giovanni.cabiddu,
giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
---
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,
--
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
2016-09-06 10:18 ` Giovanni Cabiddu
@ 2016-09-13 12:40 ` Herbert Xu
-1 siblings, 0 replies; 24+ messages in thread
From: Herbert Xu @ 2016-09-13 12:40 UTC (permalink / raw)
To: Giovanni Cabiddu
Cc: Quentin Lambert, Salvatore Benedetto, David S. Miller, qat-linux,
linux-crypto, linux-kernel, kernel-janitors, giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
@ 2016-09-13 12:40 ` Herbert Xu
0 siblings, 0 replies; 24+ messages in thread
From: Herbert Xu @ 2016-09-13 12:40 UTC (permalink / raw)
To: Giovanni Cabiddu
Cc: Quentin Lambert, Salvatore Benedetto, David S. Miller, qat-linux,
linux-crypto, linux-kernel, kernel-janitors, giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
2016-09-13 12:40 ` Herbert Xu
@ 2016-09-13 12:56 ` Quentin Lambert
-1 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-13 12:56 UTC (permalink / raw)
To: Herbert Xu, Giovanni Cabiddu
Cc: Salvatore Benedetto, David S. Miller, qat-linux, linux-crypto,
linux-kernel, kernel-janitors, giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
> Patch applied. Thanks.
Sorry, I completly missed Giovanni's message. Good work!
Quentin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] crypto: qat - fix resource release omissions
@ 2016-09-13 12:56 ` Quentin Lambert
0 siblings, 0 replies; 24+ messages in thread
From: Quentin Lambert @ 2016-09-13 12:56 UTC (permalink / raw)
To: Herbert Xu, Giovanni Cabiddu
Cc: Salvatore Benedetto, David S. Miller, qat-linux, linux-crypto,
linux-kernel, kernel-janitors, giovanni.cabiddu
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 <giovanni.cabiddu@intel.com>
> Patch applied. Thanks.
Sorry, I completly missed Giovanni's message. Good work!
Quentin
^ permalink raw reply [flat|nested] 24+ messages in thread