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:   Mon, 16 Sep 2019 13:22:03 +0000
From:   "Lendacky, Thomas" <Thomas.Lendacky@....com>
To:     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/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.

Thanks,
Tom

>  		}
>