From patchwork Thu Feb 23 10:10:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 13150039 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 46D11C636D6 for ; Thu, 23 Feb 2023 10:11:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AW7Fo10tAxfFhbSGwipjytzj2WrS4pt8eQ8R3KFqpUg=; b=KzmEI+UTMUWEwb PaPDRT+TBI1VMJNXXC9wSucEaURAUmf5+P2yCZ1rezkLYXFJKU6HVjXG+4cKL4cqxplWi0AYmM5Xt GTl/psu+h2AJlA6WmO4n6SLWrjS6g1VFfPm7YDWhDdwRJ7yybpsezyw0L7Sa1TjoQUM6xtRPSEiY6 tpVSqKOvjzsD0uBvK9VCuPwB5QNM6aL3C7mRmLQwSZwr6YMEyFzQ8SCQCSVM0l7Vj6zgU385MDVaO Yci7LF3CBoxk6p5ZFv+7Kq1y9NHrXbQ1jBbaIcs8kcxncRZYAQ6rZQjKSYocHLruvN4GrsROfCvVQ 9nqWrgJ0mrtrQ1fn1glQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pV8Yi-00Fraz-ED; Thu, 23 Feb 2023 10:10:40 +0000 Received: from 167-179-156-38.a7b39c.syd.nbn.aussiebb.net ([167.179.156.38]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pV8Yd-00FrXx-8c for linux-arm-kernel@lists.infradead.org; Thu, 23 Feb 2023 10:10:37 +0000 Received: from loth.rohan.me.apana.org.au ([192.168.167.2]) by formenos.hmeau.com with smtp (Exim 4.94.2 #2 (Debian)) id 1pV8YL-00Epv7-PF; Thu, 23 Feb 2023 18:10:18 +0800 Received: by loth.rohan.me.apana.org.au (sSMTP sendmail emulation); Thu, 23 Feb 2023 18:10:17 +0800 Date: Thu, 23 Feb 2023 18:10:17 +0800 From: Herbert Xu To: Linus Walleij Cc: Lionel Debieve , Li kunyu , davem@davemloft.net, linux-arm-kernel@lists.infradead.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, mcoquelin.stm32@gmail.com Subject: [PATCH] crypto: stm32 - Save and restore between each request Message-ID: References: <20230224231430.2948-1-kunyu@nfschina.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230223_021035_335976_9AFFD50B X-CRM114-Status: GOOD ( 24.86 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Feb 23, 2023 at 10:52:17AM +0100, Linus Walleij wrote: > > Can we fix the actual problem? It seems to have been there since the > initial submission in 2017. > > I guess the right fix is to export the *actual* hardware state into "out" > and read it back from there instead of copying out the rctx struct. > Also .statesize needs to be fixed to correspond to that. > I can just use a roof:ed constant size for this. Indeed. It looks like the driver already has everything we need but it's just in the wrong place. Here's my completely untested patch to move the hardware state reading/writing to where I think it should be. As it stands, not only is export/import broken, but multiple hashing requests occuring concurrently will overwrite each other's state. ---8<--- The Crypto API hashing paradigm requires the hardware state to be exported between *each* request because multiple unrelated hashes may be processed concurrently. The stm32 hardware is capable of producing the hardware hashing state but it was only doing it in the export function. This is not only broken for export as you can't export a kernel pointer and reimport it, but it also means that concurrent hashing was fundamentally broken. Fix this by moving the saving and restoring of hardware hash state between each and every hashing request. Fixes: 8a1012d3f2ab ("crypto: stm32 - Support for STM32 HASH module") Reported-by: Li kunyu Signed-off-by: Herbert Xu diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c index 7bf805563ac2..4bc69677a861 100644 --- a/drivers/crypto/stm32/stm32-hash.c +++ b/drivers/crypto/stm32/stm32-hash.c @@ -148,11 +148,12 @@ struct stm32_hash_request_ctx { int nents; u8 data_type; + bool started; u8 buffer[HASH_BUFLEN] __aligned(sizeof(u32)); - /* Export Context */ - u32 *hw_context; + /* hash state */ + u32 hw_context[3 + HASH_CSR_REGISTER_NUMBER]; }; struct stm32_hash_algs_info { @@ -441,8 +442,20 @@ static int stm32_hash_update_cpu(struct stm32_hash_dev *hdev) hdev->flags |= HASH_FLAGS_OUTPUT_READY; err = 0; } + } else { + u32 *preg = rctx->hw_context; + int i; + + if (!hdev->pdata->ux500) + *preg++ = stm32_hash_read(hdev, HASH_IMR); + *preg++ = stm32_hash_read(hdev, HASH_STR); + *preg++ = stm32_hash_read(hdev, HASH_CR); + for (i = 0; i < HASH_CSR_REGISTER_NUMBER; i++) + *preg++ = stm32_hash_read(hdev, HASH_CSR(i)); } + rctx->started = !final; + return err; } @@ -754,6 +767,7 @@ static int stm32_hash_init(struct ahash_request *req) rctx->total = 0; rctx->offset = 0; rctx->data_type = HASH_DATA_8_BITS; + rctx->started = false; memset(rctx->buffer, 0, HASH_BUFLEN); @@ -959,6 +973,22 @@ static int stm32_hash_one_request(struct crypto_engine *engine, void *areq) rctx = ahash_request_ctx(req); + if (rctx->started) { + u32 *preg = rctx->hw_context; + u32 reg; + int i; + + if (!hdev->pdata->ux500) + stm32_hash_write(hdev, HASH_IMR, *preg++); + stm32_hash_write(hdev, HASH_STR, *preg++); + stm32_hash_write(hdev, HASH_CR, *preg); + reg = *preg++ | HASH_CR_INIT; + stm32_hash_write(hdev, HASH_CR, reg); + + for (i = 0; i < HASH_CSR_REGISTER_NUMBER; i++) + stm32_hash_write(hdev, HASH_CSR(i), *preg++); + } + if (rctx->op == HASH_OP_UPDATE) err = stm32_hash_update_req(hdev); else if (rctx->op == HASH_OP_FINAL) @@ -1044,33 +1074,6 @@ static int stm32_hash_digest(struct ahash_request *req) static int stm32_hash_export(struct ahash_request *req, void *out) { struct stm32_hash_request_ctx *rctx = ahash_request_ctx(req); - struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req)); - struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx); - u32 *preg; - unsigned int i; - int ret; - - pm_runtime_get_sync(hdev->dev); - - ret = stm32_hash_wait_busy(hdev); - if (ret) - return ret; - - rctx->hw_context = kmalloc_array(3 + HASH_CSR_REGISTER_NUMBER, - sizeof(u32), - GFP_KERNEL); - - preg = rctx->hw_context; - - if (!hdev->pdata->ux500) - *preg++ = stm32_hash_read(hdev, HASH_IMR); - *preg++ = stm32_hash_read(hdev, HASH_STR); - *preg++ = stm32_hash_read(hdev, HASH_CR); - for (i = 0; i < HASH_CSR_REGISTER_NUMBER; i++) - *preg++ = stm32_hash_read(hdev, HASH_CSR(i)); - - pm_runtime_mark_last_busy(hdev->dev); - pm_runtime_put_autosuspend(hdev->dev); memcpy(out, rctx, sizeof(*rctx)); @@ -1080,33 +1083,9 @@ static int stm32_hash_export(struct ahash_request *req, void *out) static int stm32_hash_import(struct ahash_request *req, const void *in) { struct stm32_hash_request_ctx *rctx = ahash_request_ctx(req); - struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req)); - struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx); - const u32 *preg = in; - u32 reg; - unsigned int i; memcpy(rctx, in, sizeof(*rctx)); - preg = rctx->hw_context; - - pm_runtime_get_sync(hdev->dev); - - if (!hdev->pdata->ux500) - stm32_hash_write(hdev, HASH_IMR, *preg++); - stm32_hash_write(hdev, HASH_STR, *preg++); - stm32_hash_write(hdev, HASH_CR, *preg); - reg = *preg++ | HASH_CR_INIT; - stm32_hash_write(hdev, HASH_CR, reg); - - for (i = 0; i < HASH_CSR_REGISTER_NUMBER; i++) - stm32_hash_write(hdev, HASH_CSR(i), *preg++); - - pm_runtime_mark_last_busy(hdev->dev); - pm_runtime_put_autosuspend(hdev->dev); - - kfree(rctx->hw_context); - return 0; }