2016-01-15 14:49:06

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 0/5] crypto: atmel-sha: fix registration issue and other bugs

Hi all,

This series of patches fixes many issues such as the algo registration failure
or the broken support of context switches.

This series was applied to linux-next and tested on a sama5d2 xplained
ultra board. We now pass the tcrypt tests in the following modes:
- 2: sha1
- 6: sha256
- 11: sha384
- 12: sha512
- 33: sha224

The context switch fix was tested with a userspace program using the cryptodev
module. This single thread program computes the SHA256 hashes of many files
by splitting then into fixed size chunks. The chunks of each file are
processed by calling 'update' operations using a round robin algorithm.

However, the .import() / .export() implementation was NOT tested!
Nonetheless the last patch is needed to fix the registration issue, otherwise
atmel_sha_probe() would still fail.

Best regards,

Cyrille


Cyrille Pitchen (5):
crypto: atmel-sha: fix crash when computing digest on empty message
crypto: atmel-sha: fix a race between the 'done' tasklet and the
crypto client
crypto: atmel-sha: add support of sama5d2x SoCs
crypto: atmel-sha: fix context switches
crypto: atmel-sha: fix algorihtm registration

drivers/crypto/atmel-sha-regs.h | 4 +
drivers/crypto/atmel-sha.c | 186 +++++++++++++++++++++++++++++++++-------
2 files changed, 158 insertions(+), 32 deletions(-)

--
1.8.2.2


2016-01-15 14:49:28

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 1/5] crypto: atmel-sha: fix crash when computing digest on empty message

This patch fixes a crash which occured during the computation of the
digest of an empty message.

Indeed, when processing an empty message, the atmel_sha_handle_queue()
function was never called, hence the dd->req pointer remained
uninitialized.

Later, when the atmel_sha_final_req() function was called, it used
to crash while using this uninitialized dd->req pointer.

Hence this patch adds missing initializations of dd->req before calls of
the atmel_sha_final_req() function.

This bug prevented us from passing the tcrypt test suite on SHA algo.

Signed-off-by: Cyrille Pitchen <[email protected]>
---
drivers/crypto/atmel-sha.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 20de861aa0ea..006b2aefba30 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -939,6 +939,7 @@ static int atmel_sha_final(struct ahash_request *req)
if (err)
goto err1;

+ dd->req = req;
dd->flags |= SHA_FLAGS_BUSY;
err = atmel_sha_final_req(dd);
} else {
--
1.8.2.2

2016-01-15 14:49:32

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 2/5] crypto: atmel-sha: fix a race between the 'done' tasklet and the crypto client

The 'done' tasklet handler used to check the 'BUSY' flag to either
finalize the processing of a crypto request which had just completed or
manage the crypto queue to start the next crypto request.

On request R1 completion, the driver calls atmel_sha_finish_req(), which:
1 - clears the 'BUSY' flag since the hardware is no longer used and is
ready again to process new crypto requests.
2 - notifies the above layer (the client) about the completion of the
asynchronous crypto request R1 by calling its base.complete()
callback.
3 - schedules the 'done' task to check the crypto queue and start to
process the next crypto request (the 'BUSY' flag is supposed to be
cleared at that moment) if such a pending request exists.

However step 2 might wake the client up so it can now ask our driver to
process a new crypto request R2. This request is enqueued by calling the
atmel_sha_handle_queue() function, which sets the 'BUSY' flags then
starts to process R2.

If the 'done' tasklet, scheduled by step 3, runs just after, it would see
that the 'BUSY' flag is set then understand that R2 has just completed,
which is wrong!

So the state of 'BUSY' flag is not a proper way to detect and handle
crypto request completion.

This patch fixes this race condition by using two different tasklets, one
to handle the crypto request completion events, the other to manage the
crypto queue if needed.

Signed-off-by: Cyrille Pitchen <[email protected]>
---
drivers/crypto/atmel-sha.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 006b2aefba30..75a8cbcf3121 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -122,6 +122,7 @@ struct atmel_sha_dev {
spinlock_t lock;
int err;
struct tasklet_struct done_task;
+ struct tasklet_struct queue_task;

unsigned long flags;
struct crypto_queue queue;
@@ -788,7 +789,7 @@ static void atmel_sha_finish_req(struct ahash_request *req, int err)
req->base.complete(&req->base, err);

/* handle new request */
- tasklet_schedule(&dd->done_task);
+ tasklet_schedule(&dd->queue_task);
}

static int atmel_sha_hw_init(struct atmel_sha_dev *dd)
@@ -1101,16 +1102,18 @@ static struct ahash_alg sha_384_512_algs[] = {
},
};

+static void atmel_sha_queue_task(unsigned long data)
+{
+ struct atmel_sha_dev *dd = (struct atmel_sha_dev *)data;
+
+ atmel_sha_handle_queue(dd, NULL);
+}
+
static void atmel_sha_done_task(unsigned long data)
{
struct atmel_sha_dev *dd = (struct atmel_sha_dev *)data;
int err = 0;

- if (!(SHA_FLAGS_BUSY & dd->flags)) {
- atmel_sha_handle_queue(dd, NULL);
- return;
- }
-
if (SHA_FLAGS_CPU & dd->flags) {
if (SHA_FLAGS_OUTPUT_READY & dd->flags) {
dd->flags &= ~SHA_FLAGS_OUTPUT_READY;
@@ -1367,6 +1370,8 @@ static int atmel_sha_probe(struct platform_device *pdev)

tasklet_init(&sha_dd->done_task, atmel_sha_done_task,
(unsigned long)sha_dd);
+ tasklet_init(&sha_dd->queue_task, atmel_sha_queue_task,
+ (unsigned long)sha_dd);

crypto_init_queue(&sha_dd->queue, ATMEL_SHA_QUEUE_LENGTH);

@@ -1459,6 +1464,7 @@ err_algs:
atmel_sha_dma_cleanup(sha_dd);
err_sha_dma:
res_err:
+ tasklet_kill(&sha_dd->queue_task);
tasklet_kill(&sha_dd->done_task);
sha_dd_err:
dev_err(dev, "initialization failed.\n");
@@ -1479,6 +1485,7 @@ static int atmel_sha_remove(struct platform_device *pdev)

atmel_sha_unregister_algs(sha_dd);

+ tasklet_kill(&sha_dd->queue_task);
tasklet_kill(&sha_dd->done_task);

if (sha_dd->caps.has_dma)
--
1.8.2.2

2016-01-15 14:49:33

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 3/5] crypto: atmel-sha: add support of sama5d2x SoCs

This patch adds support of hardware version 5.1.x embedded inside sama5d2x
SoCs.

Signed-off-by: Cyrille Pitchen <[email protected]>
---
drivers/crypto/atmel-sha.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 75a8cbcf3121..7327e11e0ccb 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -1279,6 +1279,12 @@ static void atmel_sha_get_cap(struct atmel_sha_dev *dd)

/* keep only major version number */
switch (dd->hw_version & 0xff0) {
+ case 0x510:
+ dd->caps.has_dma = 1;
+ dd->caps.has_dualbuff = 1;
+ dd->caps.has_sha224 = 1;
+ dd->caps.has_sha_384_512 = 1;
+ break;
case 0x420:
dd->caps.has_dma = 1;
dd->caps.has_dualbuff = 1;
--
1.8.2.2

2016-01-15 14:49:34

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 4/5] crypto: atmel-sha: fix context switches

This patch saves the value of the internal hash register at the end of an
'update' operation then restores this value before starting the next
'update'. This way the driver can now properly handle context switches.

WARNING: only hardware versions from sama5d4x and later provide the
needed interface to update the internal hash value. Especially, sama5d3x
cannot implement this feature so context switches are still broken.

Signed-off-by: Cyrille Pitchen <[email protected]>
---
drivers/crypto/atmel-sha-regs.h | 4 ++
drivers/crypto/atmel-sha.c | 105 ++++++++++++++++++++++++++++++----------
2 files changed, 84 insertions(+), 25 deletions(-)

diff --git a/drivers/crypto/atmel-sha-regs.h b/drivers/crypto/atmel-sha-regs.h
index 83b2d7425666..e08897109cab 100644
--- a/drivers/crypto/atmel-sha-regs.h
+++ b/drivers/crypto/atmel-sha-regs.h
@@ -8,6 +8,8 @@
#define SHA_CR_START (1 << 0)
#define SHA_CR_FIRST (1 << 4)
#define SHA_CR_SWRST (1 << 8)
+#define SHA_CR_WUIHV (1 << 12)
+#define SHA_CR_WUIEHV (1 << 13)

#define SHA_MR 0x04
#define SHA_MR_MODE_MASK (0x3 << 0)
@@ -15,6 +17,8 @@
#define SHA_MR_MODE_AUTO 0x1
#define SHA_MR_MODE_PDC 0x2
#define SHA_MR_PROCDLY (1 << 4)
+#define SHA_MR_UIHV (1 << 5)
+#define SHA_MR_UIEHV (1 << 6)
#define SHA_MR_ALGO_SHA1 (0 << 8)
#define SHA_MR_ALGO_SHA256 (1 << 8)
#define SHA_MR_ALGO_SHA384 (2 << 8)
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 7327e11e0ccb..da4c3055784f 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -53,6 +53,7 @@

#define SHA_FLAGS_FINUP BIT(16)
#define SHA_FLAGS_SG BIT(17)
+#define SHA_FLAGS_ALGO_MASK GENMASK(22, 18)
#define SHA_FLAGS_SHA1 BIT(18)
#define SHA_FLAGS_SHA224 BIT(19)
#define SHA_FLAGS_SHA256 BIT(20)
@@ -60,6 +61,7 @@
#define SHA_FLAGS_SHA512 BIT(22)
#define SHA_FLAGS_ERROR BIT(23)
#define SHA_FLAGS_PAD BIT(24)
+#define SHA_FLAGS_RESTORE BIT(25)

#define SHA_OP_UPDATE 1
#define SHA_OP_FINAL 2
@@ -73,6 +75,7 @@ struct atmel_sha_caps {
bool has_dualbuff;
bool has_sha224;
bool has_sha_384_512;
+ bool has_uihv;
};

struct atmel_sha_dev;
@@ -318,7 +321,8 @@ static int atmel_sha_init(struct ahash_request *req)
static void atmel_sha_write_ctrl(struct atmel_sha_dev *dd, int dma)
{
struct atmel_sha_reqctx *ctx = ahash_request_ctx(dd->req);
- u32 valcr = 0, valmr = SHA_MR_MODE_AUTO;
+ u32 valmr = SHA_MR_MODE_AUTO;
+ unsigned int i, hashsize = 0;

if (likely(dma)) {
if (!dd->caps.has_dma)
@@ -330,22 +334,62 @@ static void atmel_sha_write_ctrl(struct atmel_sha_dev *dd, int dma)
atmel_sha_write(dd, SHA_IER, SHA_INT_DATARDY);
}

- if (ctx->flags & SHA_FLAGS_SHA1)
+ switch (ctx->flags & SHA_FLAGS_ALGO_MASK) {
+ case SHA_FLAGS_SHA1:
valmr |= SHA_MR_ALGO_SHA1;
- else if (ctx->flags & SHA_FLAGS_SHA224)
+ hashsize = SHA1_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA224:
valmr |= SHA_MR_ALGO_SHA224;
- else if (ctx->flags & SHA_FLAGS_SHA256)
+ hashsize = SHA256_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA256:
valmr |= SHA_MR_ALGO_SHA256;
- else if (ctx->flags & SHA_FLAGS_SHA384)
+ hashsize = SHA256_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA384:
valmr |= SHA_MR_ALGO_SHA384;
- else if (ctx->flags & SHA_FLAGS_SHA512)
+ hashsize = SHA512_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA512:
valmr |= SHA_MR_ALGO_SHA512;
+ hashsize = SHA512_DIGEST_SIZE;
+ break;
+
+ default:
+ break;
+ }

/* Setting CR_FIRST only for the first iteration */
- if (!(ctx->digcnt[0] || ctx->digcnt[1]))
- valcr = SHA_CR_FIRST;
+ if (!(ctx->digcnt[0] || ctx->digcnt[1])) {
+ atmel_sha_write(dd, SHA_CR, SHA_CR_FIRST);
+ } else if (dd->caps.has_uihv && (ctx->flags & SHA_FLAGS_RESTORE)) {
+ const u32 *hash = (const u32 *)ctx->digest;
+
+ /*
+ * Restore the hardware context: update the User Initialize
+ * Hash Value (UIHV) with the value saved when the latest
+ * 'update' operation completed on this very same crypto
+ * request.
+ */
+ ctx->flags &= ~SHA_FLAGS_RESTORE;
+ atmel_sha_write(dd, SHA_CR, SHA_CR_WUIHV);
+ for (i = 0; i < hashsize / sizeof(u32); ++i)
+ atmel_sha_write(dd, SHA_REG_DIN(i), hash[i]);
+ atmel_sha_write(dd, SHA_CR, SHA_CR_FIRST);
+ valmr |= SHA_MR_UIHV;
+ }
+ /*
+ * WARNING: If the UIHV feature is not available, the hardware CANNOT
+ * process concurrent requests: the internal registers used to store
+ * the hash/digest are still set to the partial digest output values
+ * computed during the latest round.
+ */

- atmel_sha_write(dd, SHA_CR, valcr);
atmel_sha_write(dd, SHA_MR, valmr);
}

@@ -714,23 +758,31 @@ static void atmel_sha_copy_hash(struct ahash_request *req)
{
struct atmel_sha_reqctx *ctx = ahash_request_ctx(req);
u32 *hash = (u32 *)ctx->digest;
- int i;
+ unsigned int i, hashsize;

- if (ctx->flags & SHA_FLAGS_SHA1)
- for (i = 0; i < SHA1_DIGEST_SIZE / sizeof(u32); i++)
- hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
- else if (ctx->flags & SHA_FLAGS_SHA224)
- for (i = 0; i < SHA224_DIGEST_SIZE / sizeof(u32); i++)
- hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
- else if (ctx->flags & SHA_FLAGS_SHA256)
- for (i = 0; i < SHA256_DIGEST_SIZE / sizeof(u32); i++)
- hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
- else if (ctx->flags & SHA_FLAGS_SHA384)
- for (i = 0; i < SHA384_DIGEST_SIZE / sizeof(u32); i++)
- hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
- else
- for (i = 0; i < SHA512_DIGEST_SIZE / sizeof(u32); i++)
- hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
+ switch (ctx->flags & SHA_FLAGS_ALGO_MASK) {
+ case SHA_FLAGS_SHA1:
+ hashsize = SHA1_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA224:
+ case SHA_FLAGS_SHA256:
+ hashsize = SHA256_DIGEST_SIZE;
+ break;
+
+ case SHA_FLAGS_SHA384:
+ case SHA_FLAGS_SHA512:
+ hashsize = SHA512_DIGEST_SIZE;
+ break;
+
+ default:
+ /* Should not happen... */
+ return;
+ }
+
+ for (i = 0; i < hashsize / sizeof(u32); ++i)
+ hash[i] = atmel_sha_read(ctx->dd, SHA_REG_DIGEST(i));
+ ctx->flags |= SHA_FLAGS_RESTORE;
}

static void atmel_sha_copy_ready_hash(struct ahash_request *req)
@@ -1276,6 +1328,7 @@ static void atmel_sha_get_cap(struct atmel_sha_dev *dd)
dd->caps.has_dualbuff = 0;
dd->caps.has_sha224 = 0;
dd->caps.has_sha_384_512 = 0;
+ dd->caps.has_uihv = 0;

/* keep only major version number */
switch (dd->hw_version & 0xff0) {
@@ -1284,12 +1337,14 @@ static void atmel_sha_get_cap(struct atmel_sha_dev *dd)
dd->caps.has_dualbuff = 1;
dd->caps.has_sha224 = 1;
dd->caps.has_sha_384_512 = 1;
+ dd->caps.has_uihv = 1;
break;
case 0x420:
dd->caps.has_dma = 1;
dd->caps.has_dualbuff = 1;
dd->caps.has_sha224 = 1;
dd->caps.has_sha_384_512 = 1;
+ dd->caps.has_uihv = 1;
break;
case 0x410:
dd->caps.has_dma = 1;
--
1.8.2.2

2016-01-15 14:49:35

by Cyrille Pitchen

[permalink] [raw]
Subject: [PATCH 5/5] crypto: atmel-sha: fix algorihtm registration

This patch implements the missing .import() and .export() mandatory
hooks for asynchronous hash algorithms. It also sets the relevant, non
zero, value for the .statesize field when declaring the supported SHA
algorithms. Indeed a zero value of .statesize prevents the algorithm from
being registered.

Signed-off-by: Cyrille Pitchen <[email protected]>
---
drivers/crypto/atmel-sha.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index da4c3055784f..ad6a0df47ccd 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -66,7 +66,7 @@
#define SHA_OP_UPDATE 1
#define SHA_OP_FINAL 2

-#define SHA_BUFFER_LEN PAGE_SIZE
+#define SHA_BUFFER_LEN (PAGE_SIZE / 16)

#define ATMEL_SHA_DMA_THRESHOLD 56

@@ -80,6 +80,17 @@ struct atmel_sha_caps {

struct atmel_sha_dev;

+/*
+ * .statesize = sizeof(struct atmel_sha_state) must be <= PAGE_SIZE / 8 as
+ * tested by the ahash_prepare_alg() function.
+ */
+struct atmel_sha_state {
+ u8 digest[SHA512_DIGEST_SIZE];
+ u8 buffer[SHA_BUFFER_LEN];
+ u64 digcnt[2];
+ size_t bufcnt;
+};
+
struct atmel_sha_reqctx {
struct atmel_sha_dev *dd;
unsigned long flags;
@@ -1033,6 +1044,33 @@ static int atmel_sha_digest(struct ahash_request *req)
return atmel_sha_init(req) ?: atmel_sha_finup(req);
}

+
+static int atmel_sha_export(struct ahash_request *req, void *out)
+{
+ const struct atmel_sha_reqctx *ctx = ahash_request_ctx(req);
+ struct atmel_sha_state *state = out;
+
+ memcpy(state->digest, ctx->digest, SHA512_DIGEST_SIZE);
+ memcpy(state->buffer, ctx->buffer, ctx->bufcnt);
+ state->bufcnt = ctx->bufcnt;
+ state->digcnt[0] = ctx->digcnt[0];
+ state->digcnt[1] = ctx->digcnt[1];
+ return 0;
+}
+
+static int atmel_sha_import(struct ahash_request *req, const void *in)
+{
+ struct atmel_sha_reqctx *ctx = ahash_request_ctx(req);
+ const struct atmel_sha_state *state = in;
+
+ memcpy(ctx->digest, state->digest, SHA512_DIGEST_SIZE);
+ memcpy(ctx->buffer, state->buffer, state->bufcnt);
+ ctx->bufcnt = state->bufcnt;
+ ctx->digcnt[0] = state->digcnt[0];
+ ctx->digcnt[1] = state->digcnt[1];
+ return 0;
+}
+
static int atmel_sha_cra_init(struct crypto_tfm *tfm)
{
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
@@ -1049,8 +1087,11 @@ static struct ahash_alg sha_1_256_algs[] = {
.final = atmel_sha_final,
.finup = atmel_sha_finup,
.digest = atmel_sha_digest,
+ .export = atmel_sha_export,
+ .import = atmel_sha_import,
.halg = {
.digestsize = SHA1_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_sha_state),
.base = {
.cra_name = "sha1",
.cra_driver_name = "atmel-sha1",
@@ -1070,8 +1111,11 @@ static struct ahash_alg sha_1_256_algs[] = {
.final = atmel_sha_final,
.finup = atmel_sha_finup,
.digest = atmel_sha_digest,
+ .export = atmel_sha_export,
+ .import = atmel_sha_import,
.halg = {
.digestsize = SHA256_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_sha_state),
.base = {
.cra_name = "sha256",
.cra_driver_name = "atmel-sha256",
@@ -1093,8 +1137,11 @@ static struct ahash_alg sha_224_alg = {
.final = atmel_sha_final,
.finup = atmel_sha_finup,
.digest = atmel_sha_digest,
+ .export = atmel_sha_export,
+ .import = atmel_sha_import,
.halg = {
.digestsize = SHA224_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_sha_state),
.base = {
.cra_name = "sha224",
.cra_driver_name = "atmel-sha224",
@@ -1116,8 +1163,11 @@ static struct ahash_alg sha_384_512_algs[] = {
.final = atmel_sha_final,
.finup = atmel_sha_finup,
.digest = atmel_sha_digest,
+ .export = atmel_sha_export,
+ .import = atmel_sha_import,
.halg = {
.digestsize = SHA384_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_sha_state),
.base = {
.cra_name = "sha384",
.cra_driver_name = "atmel-sha384",
@@ -1137,8 +1187,11 @@ static struct ahash_alg sha_384_512_algs[] = {
.final = atmel_sha_final,
.finup = atmel_sha_finup,
.digest = atmel_sha_digest,
+ .export = atmel_sha_export,
+ .import = atmel_sha_import,
.halg = {
.digestsize = SHA512_DIGEST_SIZE,
+ .statesize = sizeof(struct atmel_sha_state),
.base = {
.cra_name = "sha512",
.cra_driver_name = "atmel-sha512",
--
1.8.2.2

2016-01-22 16:52:51

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 0/5] crypto: atmel-sha: fix registration issue and other bugs

Le 15/01/2016 15:49, Cyrille Pitchen a ?crit :
> Hi all,
>
> This series of patches fixes many issues such as the algo registration failure
> or the broken support of context switches.

I think it's eligible as a "fixes" series for 4.5... Herbert, is the
whole series can be queued as fixes, in your opinion?

And, on the whole series:
Acked-by: Nicolas Ferre <[email protected]>


> This series was applied to linux-next and tested on a sama5d2 xplained
> ultra board. We now pass the tcrypt tests in the following modes:
> - 2: sha1
> - 6: sha256
> - 11: sha384
> - 12: sha512
> - 33: sha224
>
> The context switch fix was tested with a userspace program using the cryptodev
> module. This single thread program computes the SHA256 hashes of many files
> by splitting then into fixed size chunks. The chunks of each file are
> processed by calling 'update' operations using a round robin algorithm.
>
> However, the .import() / .export() implementation was NOT tested!
> Nonetheless the last patch is needed to fix the registration issue, otherwise
> atmel_sha_probe() would still fail.
>
> Best regards,
>
> Cyrille
>
>
> Cyrille Pitchen (5):
> crypto: atmel-sha: fix crash when computing digest on empty message
> crypto: atmel-sha: fix a race between the 'done' tasklet and the
> crypto client
> crypto: atmel-sha: add support of sama5d2x SoCs
> crypto: atmel-sha: fix context switches
> crypto: atmel-sha: fix algorihtm registration
>
> drivers/crypto/atmel-sha-regs.h | 4 +
> drivers/crypto/atmel-sha.c | 186 +++++++++++++++++++++++++++++++++-------
> 2 files changed, 158 insertions(+), 32 deletions(-)
>


--
Nicolas Ferre

2016-01-25 07:23:45

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 5/5] crypto: atmel-sha: fix algorihtm registration

On Fri, Jan 15, 2016 at 03:49:35PM +0100, Cyrille Pitchen wrote:
>
> +static int atmel_sha_export(struct ahash_request *req, void *out)
> +{
> + const struct atmel_sha_reqctx *ctx = ahash_request_ctx(req);
> + struct atmel_sha_state *state = out;
> +
> + memcpy(state->digest, ctx->digest, SHA512_DIGEST_SIZE);
> + memcpy(state->buffer, ctx->buffer, ctx->bufcnt);
> + state->bufcnt = ctx->bufcnt;
> + state->digcnt[0] = ctx->digcnt[0];
> + state->digcnt[1] = ctx->digcnt[1];
> + return 0;
> +}

Hmm, you're assuming that out is aligned but that is not necessarily
the case. Ditto for import.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-01-25 07:25:20

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/5] crypto: atmel-sha: fix registration issue and other bugs

On Fri, Jan 22, 2016 at 05:52:45PM +0100, Nicolas Ferre wrote:
> Le 15/01/2016 15:49, Cyrille Pitchen a ?crit :
> > Hi all,
> >
> > This series of patches fixes many issues such as the algo registration failure
> > or the broken support of context switches.
>
> I think it's eligible as a "fixes" series for 4.5... Herbert, is the
> whole series can be queued as fixes, in your opinion?

Sorry, but I'd prefer to postpone this til the next cycle as it
came in too close to the cut-off mark. This code is directly
accessible by unprivileged users so I'd like it to cook for a
bit longer.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-01-25 14:48:58

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/5] crypto: atmel-sha: fix registration issue and other bugs

On Fri, Jan 15, 2016 at 03:49:30PM +0100, Cyrille Pitchen wrote:
> Hi all,
>
> This series of patches fixes many issues such as the algo registration failure
> or the broken support of context switches.
>
> This series was applied to linux-next and tested on a sama5d2 xplained
> ultra board. We now pass the tcrypt tests in the following modes:
> - 2: sha1
> - 6: sha256
> - 11: sha384
> - 12: sha512
> - 33: sha224
>
> The context switch fix was tested with a userspace program using the cryptodev
> module. This single thread program computes the SHA256 hashes of many files
> by splitting then into fixed size chunks. The chunks of each file are
> processed by calling 'update' operations using a round robin algorithm.
>
> However, the .import() / .export() implementation was NOT tested!
> Nonetheless the last patch is needed to fix the registration issue, otherwise
> atmel_sha_probe() would still fail.

Patch 1-4 applied. Please fix the alignment issue with patch 5.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-01-25 14:56:28

by Cyrille Pitchen

[permalink] [raw]
Subject: Re: [PATCH 0/5] crypto: atmel-sha: fix registration issue and other bugs

Hi Herbert,

Le 25/01/2016 15:48, Herbert Xu a ?crit :
> On Fri, Jan 15, 2016 at 03:49:30PM +0100, Cyrille Pitchen wrote:
>> Hi all,
>>
>> This series of patches fixes many issues such as the algo registration failure
>> or the broken support of context switches.
>>
>> This series was applied to linux-next and tested on a sama5d2 xplained
>> ultra board. We now pass the tcrypt tests in the following modes:
>> - 2: sha1
>> - 6: sha256
>> - 11: sha384
>> - 12: sha512
>> - 33: sha224
>>
>> The context switch fix was tested with a userspace program using the cryptodev
>> module. This single thread program computes the SHA256 hashes of many files
>> by splitting then into fixed size chunks. The chunks of each file are
>> processed by calling 'update' operations using a round robin algorithm.
>>
>> However, the .import() / .export() implementation was NOT tested!
>> Nonetheless the last patch is needed to fix the registration issue, otherwise
>> atmel_sha_probe() would still fail.
>
> Patch 1-4 applied. Please fix the alignment issue with patch 5.
>

OK, I will fix it soon!


Best regards,

Cyrille