* [PATCH 1/2] crypto/ux500: Fix kmap() bug
2020-08-11 0:40 [PATCH 0/2] Cyrpto: Clean up kmap() use ira.weiny
@ 2020-08-11 0:40 ` ira.weiny
2020-08-11 0:40 ` [PATCH 2/2] crypto: Remove unused async iterators ira.weiny
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: ira.weiny @ 2020-08-11 0:40 UTC (permalink / raw)
To: Herbert Xu, David S. Miller; +Cc: Ira Weiny, linux-crypto, linux-kernel
From: Ira Weiny <ira.weiny@intel.com>
Once the crypto hash walk is started by crypto_hash_walk_first()
returning non-zero, crypto_hash_walk_done() must be called to unmap any
memory which was mapped by *_walk_first().
Ensure crypto_hash_walk_done() is called properly by:
1) Re-arranging the check for device data to be prior to calling
*_walk_first()
2) on error call crypto_hash_walk_done() with an error code to
allow the hash walk code to clean up.
While we are at it clean up the 'out' label to be more meaningful.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Not tested.
Found via code inspection.
---
drivers/crypto/ux500/hash/hash_core.c | 30 ++++++++++++++++-----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/ux500/hash/hash_core.c b/drivers/crypto/ux500/hash/hash_core.c
index c24f2db8d5e8..5f407a3420f8 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -1071,27 +1071,32 @@ int hash_hw_update(struct ahash_request *req)
struct hash_ctx *ctx = crypto_ahash_ctx(tfm);
struct hash_req_ctx *req_ctx = ahash_request_ctx(req);
struct crypto_hash_walk walk;
- int msg_length = crypto_hash_walk_first(req, &walk);
-
- /* Empty message ("") is correct indata */
- if (msg_length == 0)
- return ret;
+ int msg_length;
index = req_ctx->state.index;
buffer = (u8 *)req_ctx->state.buffer;
+ ret = hash_get_device_data(ctx, &device_data);
+ if (ret)
+ return ret;
+
+ msg_length = crypto_hash_walk_first(req, &walk);
+
+ /* Empty message ("") is correct indata */
+ if (msg_length == 0) {
+ ret = 0;
+ goto release_dev;
+ }
+
/* Check if ctx->state.length + msg_length
overflows */
if (msg_length > (req_ctx->state.length.low_word + msg_length) &&
HASH_HIGH_WORD_MAX_VAL == req_ctx->state.length.high_word) {
pr_err("%s: HASH_MSG_LENGTH_OVERFLOW!\n", __func__);
- return -EPERM;
+ ret = crypto_hash_walk_done(&walk, -EPERM);
+ goto release_dev;
}
- ret = hash_get_device_data(ctx, &device_data);
- if (ret)
- return ret;
-
/* Main loop */
while (0 != msg_length) {
data_buffer = walk.data;
@@ -1101,7 +1106,8 @@ int hash_hw_update(struct ahash_request *req)
if (ret) {
dev_err(device_data->dev, "%s: hash_internal_hw_update() failed!\n",
__func__);
- goto out;
+ crypto_hash_walk_done(&walk, ret);
+ goto release_dev;
}
msg_length = crypto_hash_walk_done(&walk, 0);
@@ -1111,7 +1117,7 @@ int hash_hw_update(struct ahash_request *req)
dev_dbg(device_data->dev, "%s: indata length=%d, bin=%d\n",
__func__, req_ctx->state.index, req_ctx->state.bit_index);
-out:
+release_dev:
release_hash_device(device_data);
return ret;
--
2.28.0.rc0.12.gb6a658bd00c9
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] crypto: Remove unused async iterators
2020-08-11 0:40 [PATCH 0/2] Cyrpto: Clean up kmap() use ira.weiny
2020-08-11 0:40 ` [PATCH 1/2] crypto/ux500: Fix kmap() bug ira.weiny
@ 2020-08-11 0:40 ` ira.weiny
2020-08-19 18:09 ` [PATCH 0/2] Cyrpto: Clean up kmap() use Ira Weiny
2020-08-21 7:58 ` Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: ira.weiny @ 2020-08-11 0:40 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: Ira Weiny, Ard Biesheuvel, linux-crypto, linux-kernel
From: Ira Weiny <ira.weiny@intel.com>
Revert "crypto: hash - Add real ahash walk interface"
This reverts commit 75ecb231ff45b54afa9f4ec9137965c3c00868f4.
The callers of the functions in this commit were removed in ab8085c130ed
Remove these unused calls.
Fixes: ab8085c130ed ("crypto: x86 - remove SHA multibuffer routines and mcryptd")
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
crypto/ahash.c | 41 ++++------------------------------
include/crypto/internal/hash.h | 13 -----------
2 files changed, 4 insertions(+), 50 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 68a0f0cb75c4..9c23b606949e 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -10,7 +10,6 @@
#include <crypto/internal/hash.h>
#include <crypto/scatterwalk.h>
-#include <linux/bug.h>
#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -46,10 +45,7 @@ static int hash_walk_next(struct crypto_hash_walk *walk)
unsigned int nbytes = min(walk->entrylen,
((unsigned int)(PAGE_SIZE)) - offset);
- if (walk->flags & CRYPTO_ALG_ASYNC)
- walk->data = kmap(walk->pg);
- else
- walk->data = kmap_atomic(walk->pg);
+ walk->data = kmap_atomic(walk->pg);
walk->data += offset;
if (offset & alignmask) {
@@ -99,16 +95,8 @@ int crypto_hash_walk_done(struct crypto_hash_walk *walk, int err)
}
}
- if (walk->flags & CRYPTO_ALG_ASYNC)
- kunmap(walk->pg);
- else {
- kunmap_atomic(walk->data);
- /*
- * The may sleep test only makes sense for sync users.
- * Async users don't need to sleep here anyway.
- */
- crypto_yield(walk->flags);
- }
+ kunmap_atomic(walk->data);
+ crypto_yield(walk->flags);
if (err)
return err;
@@ -140,33 +128,12 @@ int crypto_hash_walk_first(struct ahash_request *req,
walk->alignmask = crypto_ahash_alignmask(crypto_ahash_reqtfm(req));
walk->sg = req->src;
- walk->flags = req->base.flags & CRYPTO_TFM_REQ_MASK;
+ walk->flags = req->base.flags;
return hash_walk_new_entry(walk);
}
EXPORT_SYMBOL_GPL(crypto_hash_walk_first);
-int crypto_ahash_walk_first(struct ahash_request *req,
- struct crypto_hash_walk *walk)
-{
- walk->total = req->nbytes;
-
- if (!walk->total) {
- walk->entrylen = 0;
- return 0;
- }
-
- walk->alignmask = crypto_ahash_alignmask(crypto_ahash_reqtfm(req));
- walk->sg = req->src;
- walk->flags = req->base.flags & CRYPTO_TFM_REQ_MASK;
- walk->flags |= CRYPTO_ALG_ASYNC;
-
- BUILD_BUG_ON(CRYPTO_TFM_REQ_MASK & CRYPTO_ALG_ASYNC);
-
- return hash_walk_new_entry(walk);
-}
-EXPORT_SYMBOL_GPL(crypto_ahash_walk_first);
-
static int ahash_setkey_unaligned(struct crypto_ahash *tfm, const u8 *key,
unsigned int keylen)
{
diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h
index 89f6f46ab2b8..6d3ad5ac4d28 100644
--- a/include/crypto/internal/hash.h
+++ b/include/crypto/internal/hash.h
@@ -62,25 +62,12 @@ struct crypto_shash_spawn {
int crypto_hash_walk_done(struct crypto_hash_walk *walk, int err);
int crypto_hash_walk_first(struct ahash_request *req,
struct crypto_hash_walk *walk);
-int crypto_ahash_walk_first(struct ahash_request *req,
- struct crypto_hash_walk *walk);
-
-static inline int crypto_ahash_walk_done(struct crypto_hash_walk *walk,
- int err)
-{
- return crypto_hash_walk_done(walk, err);
-}
static inline int crypto_hash_walk_last(struct crypto_hash_walk *walk)
{
return !(walk->entrylen | walk->total);
}
-static inline int crypto_ahash_walk_last(struct crypto_hash_walk *walk)
-{
- return crypto_hash_walk_last(walk);
-}
-
int crypto_register_ahash(struct ahash_alg *alg);
void crypto_unregister_ahash(struct ahash_alg *alg);
int crypto_register_ahashes(struct ahash_alg *algs, int count);
--
2.28.0.rc0.12.gb6a658bd00c9
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] Cyrpto: Clean up kmap() use
2020-08-11 0:40 [PATCH 0/2] Cyrpto: Clean up kmap() use ira.weiny
2020-08-11 0:40 ` [PATCH 1/2] crypto/ux500: Fix kmap() bug ira.weiny
2020-08-11 0:40 ` [PATCH 2/2] crypto: Remove unused async iterators ira.weiny
@ 2020-08-19 18:09 ` Ira Weiny
2020-08-21 7:58 ` Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: Ira Weiny @ 2020-08-19 18:09 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-crypto, linux-kernel, Linus Walleij, Ard Biesheuvel,
Megha Dey, Tim Chen
On Mon, Aug 10, 2020 at 05:40:13PM -0700, 'Ira Weiny' wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> While going through kmap() users the following 2 issues were found via code
> inspection.
Any feedback on these patches? Perhaps I've not included the correct people?
Adding some people to the CC list.
Specifically, Linus Walleij for the ux500 work. Linus can you comment on the
first patch?
patch1: https://lore.kernel.org/lkml/20200811004015.2800392-2-ira.weiny@intel.com/
patch2: https://lore.kernel.org/lkml/20200811004015.2800392-3-ira.weiny@intel.com/
Thanks,
Ira
>
> Ira Weiny (2):
> crypto/ux500: Fix kmap() bug
> crypto: Remove unused async iterators
>
> crypto/ahash.c | 41 +++------------------------
> drivers/crypto/ux500/hash/hash_core.c | 30 ++++++++++++--------
> include/crypto/internal/hash.h | 13 ---------
> 3 files changed, 22 insertions(+), 62 deletions(-)
>
> --
> 2.28.0.rc0.12.gb6a658bd00c9
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Cyrpto: Clean up kmap() use
2020-08-11 0:40 [PATCH 0/2] Cyrpto: Clean up kmap() use ira.weiny
` (2 preceding siblings ...)
2020-08-19 18:09 ` [PATCH 0/2] Cyrpto: Clean up kmap() use Ira Weiny
@ 2020-08-21 7:58 ` Herbert Xu
3 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2020-08-21 7:58 UTC (permalink / raw)
To: ira.weiny; +Cc: David S. Miller, linux-crypto, linux-kernel
On Mon, Aug 10, 2020 at 05:40:13PM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> While going through kmap() users the following 2 issues were found via code
> inspection.
>
> Ira Weiny (2):
> crypto/ux500: Fix kmap() bug
> crypto: Remove unused async iterators
>
> crypto/ahash.c | 41 +++------------------------
> drivers/crypto/ux500/hash/hash_core.c | 30 ++++++++++++--------
> include/crypto/internal/hash.h | 13 ---------
> 3 files changed, 22 insertions(+), 62 deletions(-)
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 5+ messages in thread