2008-12-04 00:18:17

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: [PATCH 0/4] Switch remaining algorithms to shash

This series converts the remaining hash algorithms to use the new shash
interface.

The first patch removes the message schedule W from struct sha512_ctx
since it gets calculated anew on each execution of sha512_transform. This
reduces the size of sha512_ctx considerably and will allow it to be
registered as a shash algorithm (it will pass the size check in
crypto_register_shash (crypto/shash.c:490)).
Herbert, could you explain why descsize must be smaller (or equal)
than PAGE_SIZE / 8?

The next two patches switch sha512 and wp512 to the new shash interface.

The fourth patch is another try to convert michael_mic. The key values
l and r are duplicated in the descriptor part since they are used and
changed during the actual transformation. I would be gratefull for
comments on this patch since I am not sure it's the proper way to do it.

Adrian


2008-12-04 00:18:19

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: [PATCH 4/4][RFC] crypto: michael_mic - Switch to shash

This patch changes michael_mic to the new shash interface.

Signed-off-by: Adrian-Ken Rueegsegger <[email protected]>
---
crypto/michael_mic.c | 72 ++++++++++++++++++++++++++++---------------------
1 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/crypto/michael_mic.c b/crypto/michael_mic.c
index 9e917b8..079b761 100644
--- a/crypto/michael_mic.c
+++ b/crypto/michael_mic.c
@@ -9,23 +9,25 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
-
+#include <crypto/internal/hash.h>
#include <asm/byteorder.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/string.h>
-#include <linux/crypto.h>
#include <linux/types.h>


struct michael_mic_ctx {
+ u32 l, r;
+};
+
+struct michael_mic_desc_ctx {
u8 pending[4];
size_t pending_len;

u32 l, r;
};

-
static inline u32 xswap(u32 val)
{
return ((val & 0x00ff00ff) << 8) | ((val & 0xff00ff00) >> 8);
@@ -45,17 +47,22 @@ do { \
} while (0)


-static void michael_init(struct crypto_tfm *tfm)
+static int michael_init(struct shash_desc *desc)
{
- struct michael_mic_ctx *mctx = crypto_tfm_ctx(tfm);
+ struct michael_mic_desc_ctx *mctx = shash_desc_ctx(desc);
+ struct michael_mic_ctx *ctx = crypto_shash_ctx(desc->tfm);
mctx->pending_len = 0;
+ mctx->l = ctx->l;
+ mctx->r = ctx->r;
+
+ return 0;
}


-static void michael_update(struct crypto_tfm *tfm, const u8 *data,
+static int michael_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
- struct michael_mic_ctx *mctx = crypto_tfm_ctx(tfm);
+ struct michael_mic_desc_ctx *mctx = shash_desc_ctx(desc);
const __le32 *src;

if (mctx->pending_len) {
@@ -68,7 +75,7 @@ static void michael_update(struct crypto_tfm *tfm, const u8 *data,
len -= flen;

if (mctx->pending_len < 4)
- return;
+ return 0;

src = (const __le32 *)mctx->pending;
mctx->l ^= le32_to_cpup(src);
@@ -88,12 +95,14 @@ static void michael_update(struct crypto_tfm *tfm, const u8 *data,
mctx->pending_len = len;
memcpy(mctx->pending, src, len);
}
+
+ return 0;
}


-static void michael_final(struct crypto_tfm *tfm, u8 *out)
+static int michael_final(struct shash_desc *desc, u8 *out)
{
- struct michael_mic_ctx *mctx = crypto_tfm_ctx(tfm);
+ struct michael_mic_desc_ctx *mctx = shash_desc_ctx(desc);
u8 *data = mctx->pending;
__le32 *dst = (__le32 *)out;

@@ -119,17 +128,20 @@ static void michael_final(struct crypto_tfm *tfm, u8 *out)

dst[0] = cpu_to_le32(mctx->l);
dst[1] = cpu_to_le32(mctx->r);
+
+ return 0;
}


-static int michael_setkey(struct crypto_tfm *tfm, const u8 *key,
+static int michael_setkey(struct crypto_shash *tfm, const u8 *key,
unsigned int keylen)
{
- struct michael_mic_ctx *mctx = crypto_tfm_ctx(tfm);
+ struct michael_mic_ctx *mctx = crypto_shash_ctx(tfm);
+
const __le32 *data = (const __le32 *)key;

if (keylen != 8) {
- tfm->crt_flags |= CRYPTO_TFM_RES_BAD_KEY_LEN;
+ crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
return -EINVAL;
}

@@ -138,33 +150,31 @@ static int michael_setkey(struct crypto_tfm *tfm, const u8 *key,
return 0;
}

-
-static struct crypto_alg michael_mic_alg = {
- .cra_name = "michael_mic",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = 8,
- .cra_ctxsize = sizeof(struct michael_mic_ctx),
- .cra_module = THIS_MODULE,
- .cra_alignmask = 3,
- .cra_list = LIST_HEAD_INIT(michael_mic_alg.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = 8,
- .dia_init = michael_init,
- .dia_update = michael_update,
- .dia_final = michael_final,
- .dia_setkey = michael_setkey } }
+static struct shash_alg alg = {
+ .digestsize = 8,
+ .setkey = michael_setkey,
+ .init = michael_init,
+ .update = michael_update,
+ .final = michael_final,
+ .descsize = sizeof(struct michael_mic_desc_ctx),
+ .base = {
+ .cra_name = "michael_mic",
+ .cra_blocksize = 8,
+ .cra_alignmask = 3,
+ .cra_ctxsize = sizeof(struct michael_mic_ctx),
+ .cra_module = THIS_MODULE,
+ }
};


2008-12-04 00:18:18

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: [PATCH 3/4] crypto: wp512 - Switch to shash

This patch changes wp512, wp384 and wp256 to the new shash interface.

Signed-off-by: Adrian-Ken Rueegsegger <[email protected]>
---
crypto/wp512.c | 121 ++++++++++++++++++++++++++++++--------------------------
1 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/crypto/wp512.c b/crypto/wp512.c
index bff2856..7234272 100644
--- a/crypto/wp512.c
+++ b/crypto/wp512.c
@@ -19,11 +19,11 @@
* (at your option) any later version.
*
*/
+#include <crypto/internal/hash.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/mm.h>
#include <asm/byteorder.h>
-#include <linux/crypto.h>
#include <linux/types.h>

#define WP512_DIGEST_SIZE 64
@@ -980,8 +980,8 @@ static void wp512_process_buffer(struct wp512_ctx *wctx) {

}

-static void wp512_init(struct crypto_tfm *tfm) {
- struct wp512_ctx *wctx = crypto_tfm_ctx(tfm);
+static int wp512_init(struct shash_desc *desc) {
+ struct wp512_ctx *wctx = shash_desc_ctx(desc);
int i;

memset(wctx->bitLength, 0, 32);
@@ -990,12 +990,14 @@ static void wp512_init(struct crypto_tfm *tfm) {
for (i = 0; i < 8; i++) {
wctx->hash[i] = 0L;
}
+
+ return 0;
}

-static void wp512_update(struct crypto_tfm *tfm, const u8 *source,
+static int wp512_update(struct shash_desc *desc, const u8 *source,
unsigned int len)
{
- struct wp512_ctx *wctx = crypto_tfm_ctx(tfm);
+ struct wp512_ctx *wctx = shash_desc_ctx(desc);
int sourcePos = 0;
unsigned int bits_len = len * 8; // convert to number of bits
int sourceGap = (8 - ((int)bits_len & 7)) & 7;
@@ -1051,11 +1053,12 @@ static void wp512_update(struct crypto_tfm *tfm, const u8 *source,
wctx->bufferBits = bufferBits;
wctx->bufferPos = bufferPos;

+ return 0;
}

-static void wp512_final(struct crypto_tfm *tfm, u8 *out)
+static int wp512_final(struct shash_desc *desc, u8 *out)
{
- struct wp512_ctx *wctx = crypto_tfm_ctx(tfm);
+ struct wp512_ctx *wctx = shash_desc_ctx(desc);
int i;
u8 *buffer = wctx->buffer;
u8 *bitLength = wctx->bitLength;
@@ -1084,89 +1087,95 @@ static void wp512_final(struct crypto_tfm *tfm, u8 *out)
digest[i] = cpu_to_be64(wctx->hash[i]);
wctx->bufferBits = bufferBits;
wctx->bufferPos = bufferPos;
+
+ return 0;
}

-static void wp384_final(struct crypto_tfm *tfm, u8 *out)
+static int wp384_final(struct shash_desc *desc, u8 *out)
{
u8 D[64];

- wp512_final(tfm, D);
+ wp512_final(desc, D);
memcpy (out, D, WP384_DIGEST_SIZE);
memset (D, 0, WP512_DIGEST_SIZE);
+
+ return 0;
}

-static void wp256_final(struct crypto_tfm *tfm, u8 *out)
+static int wp256_final(struct shash_desc *desc, u8 *out)
{
u8 D[64];

- wp512_final(tfm, D);
+ wp512_final(desc, D);
memcpy (out, D, WP256_DIGEST_SIZE);
memset (D, 0, WP512_DIGEST_SIZE);
+
+ return 0;
}

-static struct crypto_alg wp512 = {
- .cra_name = "wp512",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = WP512_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct wp512_ctx),
- .cra_module = THIS_MODULE,
- .cra_list = LIST_HEAD_INIT(wp512.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = WP512_DIGEST_SIZE,
- .dia_init = wp512_init,
- .dia_update = wp512_update,
- .dia_final = wp512_final } }
+static struct shash_alg wp512 = {
+ .digestsize = WP512_DIGEST_SIZE,
+ .init = wp512_init,
+ .update = wp512_update,
+ .final = wp512_final,
+ .descsize = sizeof(struct wp512_ctx),
+ .base = {
+ .cra_name = "wp512",
+ .cra_flags = CRYPTO_ALG_TYPE_SHASH,
+ .cra_blocksize = WP512_BLOCK_SIZE,
+ .cra_module = THIS_MODULE,
+ }
};

-static struct crypto_alg wp384 = {
- .cra_name = "wp384",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = WP512_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct wp512_ctx),
- .cra_module = THIS_MODULE,
- .cra_list = LIST_HEAD_INIT(wp384.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = WP384_DIGEST_SIZE,
- .dia_init = wp512_init,
- .dia_update = wp512_update,
- .dia_final = wp384_final } }
+static struct shash_alg wp384 = {
+ .digestsize = WP384_DIGEST_SIZE,
+ .init = wp512_init,
+ .update = wp512_update,
+ .final = wp384_final,
+ .descsize = sizeof(struct wp512_ctx),
+ .base = {
+ .cra_name = "wp384",
+ .cra_flags = CRYPTO_ALG_TYPE_SHASH,
+ .cra_blocksize = WP512_BLOCK_SIZE,
+ .cra_module = THIS_MODULE,
+ }
};

-static struct crypto_alg wp256 = {
- .cra_name = "wp256",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = WP512_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct wp512_ctx),
- .cra_module = THIS_MODULE,
- .cra_list = LIST_HEAD_INIT(wp256.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = WP256_DIGEST_SIZE,
- .dia_init = wp512_init,
- .dia_update = wp512_update,
- .dia_final = wp256_final } }
+static struct shash_alg wp256 = {
+ .digestsize = WP256_DIGEST_SIZE,
+ .init = wp512_init,
+ .update = wp512_update,
+ .final = wp256_final,
+ .descsize = sizeof(struct wp512_ctx),
+ .base = {
+ .cra_name = "wp256",
+ .cra_flags = CRYPTO_ALG_TYPE_SHASH,
+ .cra_blocksize = WP512_BLOCK_SIZE,
+ .cra_module = THIS_MODULE,
+ }
};

static int __init wp512_mod_init(void)
{
int ret = 0;

- ret = crypto_register_alg(&wp512);
+ ret = crypto_register_shash(&wp512);

if (ret < 0)
goto out;

- ret = crypto_register_alg(&wp384);
+ ret = crypto_register_shash(&wp384);
if (ret < 0)
{
- crypto_unregister_alg(&wp512);
+ crypto_unregister_shash(&wp512);
goto out;
}

- ret = crypto_register_alg(&wp256);
+ ret = crypto_register_shash(&wp256);
if (ret < 0)
{
- crypto_unregister_alg(&wp512);
- crypto_unregister_alg(&wp384);
+ crypto_unregister_shash(&wp512);
+ crypto_unregister_shash(&wp384);
}
out:
return ret;
@@ -1174,9 +1183,9 @@ out:

static void __exit wp512_mod_fini(void)
{
- crypto_unregister_alg(&wp512);
- crypto_unregister_alg(&wp384);
- crypto_unregister_alg(&wp256);
+ crypto_unregister_shash(&wp512);
+ crypto_unregister_shash(&wp384);
+ crypto_unregister_shash(&wp256);
}

MODULE_ALIAS("wp384");
--
1.5.2.5


2008-12-04 00:18:17

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: [PATCH 1/4] crypto: sha512 - Remove W (message schedule) from struct sha512_ctx

The message schedule W[80] is calculated anew when sha512_transform
is executed. Therefore it is local to that function and does not need
to be defined in struct sha512_ctx.
Note: the sha256 algorithm already does it this way.

Signed-off-by: Adrian-Ken Rueegsegger <[email protected]>
---
crypto/sha512_generic.c | 13 +++++--------
1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/crypto/sha512_generic.c b/crypto/sha512_generic.c
index bc36861..e0b0303 100644
--- a/crypto/sha512_generic.c
+++ b/crypto/sha512_generic.c
@@ -25,7 +25,6 @@ struct sha512_ctx {
u64 state[8];
u32 count[4];
u8 buf[128];
- u64 W[80];
};

static inline u64 Ch(u64 x, u64 y, u64 z)
@@ -89,10 +88,10 @@ static inline void BLEND_OP(int I, u64 *W)
}

static void
-sha512_transform(u64 *state, u64 *W, const u8 *input)
+sha512_transform(u64 *state, const u8 *input)
{
u64 a, b, c, d, e, f, g, h, t1, t2;
-
+ u64 W[80];
int i;

/* load the input */
@@ -132,6 +131,7 @@ sha512_transform(u64 *state, u64 *W, const u8 *input)

/* erase our data */
a = b = c = d = e = f = g = h = t1 = t2 = 0;
+ memset(W, 0, 80 * sizeof(u64));
}

static void
@@ -187,10 +187,10 @@ sha512_update(struct crypto_tfm *tfm, const u8 *data, unsigned int len)
/* Transform as many times as possible. */
if (len >= part_len) {
memcpy(&sctx->buf[index], data, part_len);
- sha512_transform(sctx->state, sctx->W, sctx->buf);
+ sha512_transform(sctx->state, sctx->buf);

for (i = part_len; i + 127 < len; i+=128)
- sha512_transform(sctx->state, sctx->W, &data[i]);
+ sha512_transform(sctx->state, &data[i]);

index = 0;
} else {
@@ -199,9 +199,6 @@ sha512_update(struct crypto_tfm *tfm, const u8 *data, unsigned int len)

/* Buffer remaining input */
memcpy(&sctx->buf[index], &data[i], len - i);

2008-12-04 00:18:18

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: [PATCH 2/4] crypto: sha512 - Switch to shash

This patch changes sha512 and sha384 to the new shash interface.

Signed-off-by: Adrian-Ken Rueegsegger <[email protected]>
---
crypto/sha512_generic.c | 107 ++++++++++++++++++++++++----------------------
1 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/crypto/sha512_generic.c b/crypto/sha512_generic.c
index e0b0303..2834e55 100644
--- a/crypto/sha512_generic.c
+++ b/crypto/sha512_generic.c
@@ -10,12 +10,11 @@
* later version.
*
*/
-
+#include <crypto/internal/hash.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mm.h>
#include <linux/init.h>
-#include <linux/crypto.h>
#include <linux/types.h>
#include <crypto/sha.h>

@@ -134,10 +133,10 @@ sha512_transform(u64 *state, const u8 *input)
memset(W, 0, 80 * sizeof(u64));
}

-static void
-sha512_init(struct crypto_tfm *tfm)
+static int
+sha512_init(struct shash_desc *desc)
{
- struct sha512_ctx *sctx = crypto_tfm_ctx(tfm);
+ struct sha512_ctx *sctx = shash_desc_ctx(desc);
sctx->state[0] = SHA512_H0;
sctx->state[1] = SHA512_H1;
sctx->state[2] = SHA512_H2;
@@ -147,12 +146,14 @@ sha512_init(struct crypto_tfm *tfm)
sctx->state[6] = SHA512_H6;
sctx->state[7] = SHA512_H7;
sctx->count[0] = sctx->count[1] = sctx->count[2] = sctx->count[3] = 0;
+
+ return 0;
}

-static void
-sha384_init(struct crypto_tfm *tfm)
+static int
+sha384_init(struct shash_desc *desc)
{
- struct sha512_ctx *sctx = crypto_tfm_ctx(tfm);
+ struct sha512_ctx *sctx = shash_desc_ctx(desc);
sctx->state[0] = SHA384_H0;
sctx->state[1] = SHA384_H1;
sctx->state[2] = SHA384_H2;
@@ -162,12 +163,14 @@ sha384_init(struct crypto_tfm *tfm)
sctx->state[6] = SHA384_H6;
sctx->state[7] = SHA384_H7;
sctx->count[0] = sctx->count[1] = sctx->count[2] = sctx->count[3] = 0;
+
+ return 0;
}

-static void
-sha512_update(struct crypto_tfm *tfm, const u8 *data, unsigned int len)
+static int
+sha512_update(struct shash_desc *desc, const u8 *data, unsigned int len)
{
- struct sha512_ctx *sctx = crypto_tfm_ctx(tfm);
+ struct sha512_ctx *sctx = shash_desc_ctx(desc);

unsigned int i, index, part_len;

@@ -199,12 +202,14 @@ sha512_update(struct crypto_tfm *tfm, const u8 *data, unsigned int len)

/* Buffer remaining input */
memcpy(&sctx->buf[index], &data[i], len - i);
+
+ return 0;
}

-static void
-sha512_final(struct crypto_tfm *tfm, u8 *hash)
+static int
+sha512_final(struct shash_desc *desc, u8 *hash)
{
- struct sha512_ctx *sctx = crypto_tfm_ctx(tfm);
+ struct sha512_ctx *sctx = shash_desc_ctx(desc);
static u8 padding[128] = { 0x80, };
__be64 *dst = (__be64 *)hash;
__be32 bits[4];
@@ -220,10 +225,10 @@ sha512_final(struct crypto_tfm *tfm, u8 *hash)
/* Pad out to 112 mod 128. */
index = (sctx->count[0] >> 3) & 0x7f;
pad_len = (index < 112) ? (112 - index) : ((128+112) - index);
- sha512_update(tfm, padding, pad_len);
+ sha512_update(desc, padding, pad_len);

/* Append length (before padding) */
- sha512_update(tfm, (const u8 *)bits, sizeof(bits));
+ sha512_update(desc, (const u8 *)bits, sizeof(bits));

/* Store state in digest */
for (i = 0; i < 8; i++)
@@ -231,66 +236,66 @@ sha512_final(struct crypto_tfm *tfm, u8 *hash)

/* Zeroize sensitive information. */
memset(sctx, 0, sizeof(struct sha512_ctx));
+
+ return 0;
}

-static void sha384_final(struct crypto_tfm *tfm, u8 *hash)
+static int sha384_final(struct shash_desc *desc, u8 *hash)
{
u8 D[64];

- sha512_final(tfm, D);
+ sha512_final(desc, D);

memcpy(hash, D, 48);
memset(D, 0, 64);
+
+ return 0;
}

-static struct crypto_alg sha512 = {
- .cra_name = "sha512",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = SHA512_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct sha512_ctx),
- .cra_module = THIS_MODULE,
- .cra_alignmask = 3,
- .cra_list = LIST_HEAD_INIT(sha512.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = SHA512_DIGEST_SIZE,
- .dia_init = sha512_init,
- .dia_update = sha512_update,
- .dia_final = sha512_final }
- }
+static struct shash_alg sha512 = {
+ .digestsize = SHA512_DIGEST_SIZE,
+ .init = sha512_init,
+ .update = sha512_update,
+ .final = sha512_final,
+ .descsize = sizeof(struct sha512_ctx),
+ .base = {
+ .cra_name = "sha512",
+ .cra_flags = CRYPTO_ALG_TYPE_SHASH,
+ .cra_blocksize = SHA512_BLOCK_SIZE,
+ .cra_module = THIS_MODULE,
+ }
};

-static struct crypto_alg sha384 = {
- .cra_name = "sha384",
- .cra_flags = CRYPTO_ALG_TYPE_DIGEST,
- .cra_blocksize = SHA384_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct sha512_ctx),
- .cra_alignmask = 3,
- .cra_module = THIS_MODULE,
- .cra_list = LIST_HEAD_INIT(sha384.cra_list),
- .cra_u = { .digest = {
- .dia_digestsize = SHA384_DIGEST_SIZE,
- .dia_init = sha384_init,
- .dia_update = sha512_update,
- .dia_final = sha384_final }
- }
+static struct shash_alg sha384 = {
+ .digestsize = SHA384_DIGEST_SIZE,
+ .init = sha384_init,
+ .update = sha512_update,
+ .final = sha384_final,
+ .descsize = sizeof(struct sha512_ctx),
+ .base = {
+ .cra_name = "sha384",
+ .cra_flags = CRYPTO_ALG_TYPE_SHASH,
+ .cra_blocksize = SHA384_BLOCK_SIZE,
+ .cra_module = THIS_MODULE,
+ }
};

static int __init sha512_generic_mod_init(void)
{
int ret = 0;

- if ((ret = crypto_register_alg(&sha384)) < 0)
+ if ((ret = crypto_register_shash(&sha384)) < 0)
goto out;
- if ((ret = crypto_register_alg(&sha512)) < 0)
- crypto_unregister_alg(&sha384);
+ if ((ret = crypto_register_shash(&sha512)) < 0)
+ crypto_unregister_shash(&sha384);
out:
return ret;
}

static void __exit sha512_generic_mod_fini(void)
{
- crypto_unregister_alg(&sha384);
- crypto_unregister_alg(&sha512);
+ crypto_unregister_shash(&sha384);
+ crypto_unregister_shash(&sha512);
}

module_init(sha512_generic_mod_init);
--
1.5.2.5


2008-12-04 03:37:22

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 4/4][RFC] crypto: michael_mic - Switch to shash

On Thu, Dec 04, 2008 at 01:18:12AM +0100, Adrian-Ken Rueegsegger wrote:
>
> struct michael_mic_ctx {
> + u32 l, r;
> +};
> +
> +struct michael_mic_desc_ctx {
> u8 pending[4];
> size_t pending_len;
>
> u32 l, r;
> };

Any reason why you left them in the desc context?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-04 06:48:15

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/4] Switch remaining algorithms to shash

On Thu, Dec 04, 2008 at 01:18:08AM +0100, Adrian-Ken Rueegsegger wrote:
>
> The first patch removes the message schedule W from struct sha512_ctx
> since it gets calculated anew on each execution of sha512_transform. This
> reduces the size of sha512_ctx considerably and will allow it to be
> registered as a shash algorithm (it will pass the size check in
> crypto_register_shash (crypto/shash.c:490)).
> Herbert, could you explain why descsize must be smaller (or equal)
> than PAGE_SIZE / 8?

This is so that people can put it on the stack safely. So moving
things out of the context and onto the stack because it's too big
is a no-no :)

Perhaps store in a static percpu area?

> The next two patches switch sha512 and wp512 to the new shash interface.

BTW, in order to add missing Kconfig dependencies on HASH I've
just rebased my tree and updated all the shash conversion patches.
So please resend them with the Kconfig bits added.

> The fourth patch is another try to convert michael_mic. The key values
> l and r are duplicated in the descriptor part since they are used and
> changed during the actual transformation. I would be gratefull for
> comments on this patch since I am not sure it's the proper way to do it.

Since they're read-only they should be obtained from the tfm context
when needed, just like crc32c.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-04 07:56:08

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: Re: [PATCH 0/4] Switch remaining algorithms to shash

Hello Herbert,

Herbert Xu wrote:
> On Thu, Dec 04, 2008 at 01:18:08AM +0100, Adrian-Ken Rueegsegger wrote:
>> The first patch removes the message schedule W from struct sha512_ctx
>> since it gets calculated anew on each execution of sha512_transform. This
>> reduces the size of sha512_ctx considerably and will allow it to be
>> registered as a shash algorithm (it will pass the size check in
>> crypto_register_shash (crypto/shash.c:490)).
>> Herbert, could you explain why descsize must be smaller (or equal)
>> than PAGE_SIZE / 8?
>
> This is so that people can put it on the stack safely. So moving
> things out of the context and onto the stack because it's too big
> is a no-no :)

Thanks for the explanation.

> Perhaps store in a static percpu area?
>
>> The next two patches switch sha512 and wp512 to the new shash interface.
>
> BTW, in order to add missing Kconfig dependencies on HASH I've
> just rebased my tree and updated all the shash conversion patches.
> So please resend them with the Kconfig bits added.

Will do. I will resubmit the patches later today.

>> The fourth patch is another try to convert michael_mic. The key values
>> l and r are duplicated in the descriptor part since they are used and
>> changed during the actual transformation. I would be gratefull for
>> comments on this patch since I am not sure it's the proper way to do it.
>
> Since they're read-only they should be obtained from the tfm context
> when needed, just like crc32c.

If I read the code correctly l and r are not read-only, e.g. in
michael_update there are multiple assignments to mctx->l and mctx->r.
That's the reason why I left them in the desc context.

Thank you for your comments,
Adrian

2008-12-04 08:02:35

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/4] Switch remaining algorithms to shash

On Thu, Dec 04, 2008 at 08:55:57AM +0100, Adrian-Ken Rueegsegger wrote:
>
> If I read the code correctly l and r are not read-only, e.g. in
> michael_update there are multiple assignments to mctx->l and mctx->r.
> That's the reason why I left them in the desc context.

I see. In that case your patch is correct.

This also means that the original michael_mic semantics broke
the crypto API requirement that setkey be persistent across the
life-time of an tfm (at least until the next setkey). Luckily
it seems that the michael_mic users always perform a setkey
prior to each operation so this is OK.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-12-04 08:04:41

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: Re: [PATCH 4/4][RFC] crypto: michael_mic - Switch to shash

Herbert Xu wrote:
> On Thu, Dec 04, 2008 at 01:18:12AM +0100, Adrian-Ken Rueegsegger wrote:
>> struct michael_mic_ctx {
>> + u32 l, r;
>> +};
>> +
>> +struct michael_mic_desc_ctx {
>> u8 pending[4];
>> size_t pending_len;
>>
>> u32 l, r;
>> };
>
> Any reason why you left them in the desc context?

As I explained in the other mail, the values l and r are not readonly.
Therefor I believe they need to be copied from the tfm context to the
descriptor upon transformation.

Adrian