mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Lionel Debieve <lionel.debieve@st.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@st.com>,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] crypto: stm32 - Try to fix hash padding
Date: Tue, 12 Sep 2017 11:35:39 +0200	[thread overview]
Message-ID: <20170912093553.2580986-2-arnd@arndb.de> (raw)
In-Reply-To: <20170912093553.2580986-1-arnd@arndb.de>

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@arndb.de>
---
 drivers/crypto/stm32/stm32-hash.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
index 3c23a23e9ee5..b2ed2bccdb66 100644
--- a/drivers/crypto/stm32/stm32-hash.c
+++ b/drivers/crypto/stm32/stm32-hash.c
@@ -553,9 +553,9 @@ static int stm32_hash_dma_send(struct stm32_hash_dev *hdev)
 {
 	struct stm32_hash_request_ctx *rctx = ahash_request_ctx(hdev->req);
 	struct scatterlist sg[1], *tsg;
-	int err = 0, len = 0, reg, ncp;
+	int err = 0, len = 0, reg, ncp = 0;
 	unsigned int i;
-	const u32 *buffer = (const u32 *)rctx->buffer;
+	u32 *buffer = (void *)rctx->buffer;
 
 	rctx->sg = hdev->req->src;
 	rctx->total = hdev->req->nbytes;
@@ -620,10 +620,13 @@ static int stm32_hash_dma_send(struct stm32_hash_dev *hdev)
 		reg |= HASH_CR_DMAA;
 		stm32_hash_write(hdev, HASH_CR, reg);
 
-		for (i = 0; i < DIV_ROUND_UP(ncp, sizeof(u32)); i++)
-			stm32_hash_write(hdev, HASH_DIN, buffer[i]);
-
-		stm32_hash_set_nblw(hdev, ncp);
+		if (ncp) {
+			memset(buffer + ncp, 0,
+			       DIV_ROUND_UP(ncp, sizeof(u32)) - ncp);
+			writesl(hdev->io_base + HASH_DIN, buffer,
+				DIV_ROUND_UP(ncp, sizeof(u32)));
+		}
+		stm32_hash_set_nblw(hdev, DIV_ROUND_UP(ncp, sizeof(u32)));
 		reg = stm32_hash_read(hdev, HASH_STR);
 		reg |= HASH_STR_DCAL;
 		stm32_hash_write(hdev, HASH_STR, reg);
-- 
2.9.0

  reply	other threads:[~2017-09-12  9:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-12  9:35 [PATCH 1/2] crypto: stm32 - Fix uninitialized data usage Arnd Bergmann
2017-09-12  9:35 ` Arnd Bergmann [this message]
2017-10-07  4:20   ` [PATCH 2/2] crypto: stm32 - Try to fix hash padding Herbert Xu
     [not found] ` <b04df870-3c15-65ea-1f3e-d9a4338ff485@st.com>
2017-09-18  9:36   ` [PATCH 1/2] crypto: stm32 - Fix uninitialized data usage Lionel DEBIEVE
2017-10-07  4:19 ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170912093553.2580986-2-arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=alexandre.torgue@st.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lionel.debieve@st.com \
    --cc=mcoquelin.stm32@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome