linux-kernel - Re: [PATCH] crypto: ccp - release hmac_buf if ccp_run_sha

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Sep 2019 13:29:17 +0000
From:   Gary R Hook <ghook@....com>
To:     "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        Navid Emamdoost <navid.emamdoost@...il.com>
CC:     "emamd001@....edu" <emamd001@....edu>,
        "smccaman@....edu" <smccaman@....edu>,
        "kjlu@....edu" <kjlu@....edu>, "Hook, Gary" <Gary.Hook@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: ccp - release hmac_buf if ccp_run_sha_cmd fails

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@...il.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