| Message ID | 20190913234824.8521-1-navid.emamdoost@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: ccp - release hmac_buf if ccp_run_sha_cmd fails | expand |
On 9/13/19 6:48 PM, Navid Emamdoost wrote: > In ccp_run_sha_cmd, if the type of sha is invalid, the allocated > hmac_buf should be released. > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> > --- > drivers/crypto/ccp/ccp-ops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c > index 9bc3c62157d7..cff16f0cc15b 100644 > --- a/drivers/crypto/ccp/ccp-ops.c > +++ b/drivers/crypto/ccp/ccp-ops.c > @@ -1782,6 +1782,7 @@ static int ccp_run_sha_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd) > LSB_ITEM_SIZE); > break; > default: > + kfree(hmac_buf); Well, theoretically we can never reach this section since the routine would have never proceeded past the first switch statement at the beginning of the function. But, if the code is ever modified and some of the switch statements missed then it's possible... > ret = -EINVAL; > goto e_ctx; I know it's not part of your change, but this looks like it should be goto e_data instead of e_ctx, too. Thanks, Tom > } >
On 9/16/19 8:22 AM, Lendacky, Thomas wrote: > On 9/13/19 6:48 PM, Navid Emamdoost wrote: >> In ccp_run_sha_cmd, if the type of sha is invalid, the allocated >> hmac_buf should be released. >> >> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> >> --- >> drivers/crypto/ccp/ccp-ops.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c >> index 9bc3c62157d7..cff16f0cc15b 100644 >> --- a/drivers/crypto/ccp/ccp-ops.c >> +++ b/drivers/crypto/ccp/ccp-ops.c >> @@ -1782,6 +1782,7 @@ static int ccp_run_sha_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd) >> LSB_ITEM_SIZE); >> break; >> default: >> + kfree(hmac_buf); > Well, theoretically we can never reach this section since the routine > would have never proceeded past the first switch statement at the > beginning of the function. But, if the code is ever modified and some of > the switch statements missed then it's possible... > >> ret = -EINVAL; >> goto e_ctx; > I know it's not part of your change, but this looks like it should be > goto e_data instead of e_ctx, too. I agree with this. Please resubmit with the suggested change, and use a commit message along the the lines of crypto: ccp - Release all allocated memory if sha type is invalid Gary
diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c index 9bc3c62157d7..cff16f0cc15b 100644 --- a/drivers/crypto/ccp/ccp-ops.c +++ b/drivers/crypto/ccp/ccp-ops.c @@ -1782,6 +1782,7 @@ static int ccp_run_sha_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd) LSB_ITEM_SIZE); break; default: + kfree(hmac_buf); ret = -EINVAL; goto e_ctx; }
In ccp_run_sha_cmd, if the type of sha is invalid, the allocated hmac_buf should be released. Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> --- drivers/crypto/ccp/ccp-ops.c | 1 + 1 file changed, 1 insertion(+)