linux-kernel - Re: [PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos

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] [day] [month] [year] [list]

Message-ID: <12d1674b-c1a0-6a9b-644e-e0a40fbb4457@mleia.com>
Date:   Wed, 25 Oct 2017 07:32:46 +0300
From:   Vladimir Zapolskiy <vz@...ia.com>
To:     Kamil Konieczny <k.konieczny@...tner.samsung.com>,
        linux-crypto@...r.kernel.org
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos

Hi Kamil,

I'll just answer to your question, all the comments from you are accepted,
please send a new version with the minor fixes, hopefully the change will
be included into v4.15-rc.

On 10/24/2017 02:27 PM, Kamil Konieczny wrote:
> Hi Vladimir,
> 
> Thank you for review, I will apply almost all of your remarks,
> see answers below.
> 
> On 22.10.2017 12:18, Vladimir Zapolskiy wrote:
>> Hi Kamil,
>>
>> thank you for updates, I have just a few more comments.
>>

[snip]

>>> +/**
>>> + * s5p_hash_import - import hash state
>>> + * @req:	AHASH request
>>> + * @in:		buffer with state to be imported from
>>> + */
>>> +static int s5p_hash_import(struct ahash_request *req, const void *in)
>>> +{
>>> +	struct s5p_hash_reqctx *ctx = ahash_request_ctx(req);
>>> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
>>> +	struct s5p_hash_ctx *tctx = crypto_ahash_ctx(tfm);
>>> +	const struct s5p_hash_reqctx *ctx_in = in;
>>> +
>>> +	memcpy(ctx, in, sizeof(*ctx) + BUFLEN);
>>> +	if (ctx_in->bufcnt < 0 || ctx_in->bufcnt > BUFLEN) {
>>
>> Now "ctx_in->bufcnt < 0" condition can be removed, moreover it
>> will eliminate a warning reproted by the compiler:
>>
>> drivers/crypto/s5p-sss.c:1748:21: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>   if (ctx_in->bufcnt < 0 || ctx_in->bufcnt > BUFLEN) {
>>                      ^
> 
> You are right, I will drop first condition. BTW what compiler options are you using ?
> This one was not reported by my compilation process.
> 

Regularly I specify 'make C=1 W=1' options to build a kernel with changes,
some of the new reported warnings are false positives, but in general it
makes sense to check the output to catch potential issues like this one.

--
With best wishes,
Vladimir