linux-kernel - Re: [PATCH 2/2] crypto: stm32

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]

Date:   Sat, 7 Oct 2017 12:20:20 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Lionel Debieve <lionel.debieve@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        linux-crypto@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] crypto: stm32 - Try to fix hash padding

On Tue, Sep 12, 2017 at 11:35:39AM +0200, Arnd Bergmann wrote:
> gcc warns that the length for the extra unaligned data in the hash
> function may be used unaligned. In theory this could happen if
> we pass a zero-length sg_list, or if sg_is_last() was never true:
> 
> In file included from drivers/crypto/stm32/stm32-hash.c:23:
> drivers/crypto/stm32/stm32-hash.c: In function 'stm32_hash_one_request':
> include/uapi/linux/kernel.h:12:49: error: 'ncp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>  #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> 
> Neither of these can happen in practice, so the warning is harmless.
> 
> However while trying to suppress the warning, I noticed multiple
> problems with that code:
> 
> - On big-endian kernels, we byte-swap the data like we do for
>   register accesses, however this is a data stream and almost
>   certainly needs to use a single writesl() instead of series
>   of writel() to give the correct hash.
> 
> - If the length is not a multiple of four bytes, we skip the
>   last word entirely, since we write the truncated length
>   using stm32_hash_set_nblw().
> 
> - If we change the code to round the length up rather than
>   down, the last bytes contain stale data, so it needs some
>   form of padding.
> 
> This tries to address all four problems, by correctly
> initializing the length to zero, using endian-safe copy
> functions, adding zero-padding and passing the padded length.
> 
> I have done no testing on this patch, so please review
> carefully and if possible test with an unaligned length
> and big-endian kernel builds.
> 
> Fixes: 8a1012d3f2ab ("crypto: stm32 - Support for STM32 HASH module")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt