* [PATCH] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE
@ 2011-05-26 3:11 Mandeep Singh Baines
2011-05-26 3:34 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Mandeep Singh Baines @ 2011-05-26 3:11 UTC (permalink / raw)
To: linux-kernel
Cc: Mandeep Singh Baines, Herbert Xu, David S. Miller, linux-crypto
Plus some other minor cleanup.
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
---
crypto/sha1_generic.c | 39 +++++++++++++++++++--------------------
1 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..4bdd228 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -40,33 +40,32 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
- unsigned int partial, done;
- const u8 *src;
+ unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
+ u32 temp[SHA_WORKSPACE_WORDS];
+ const u8 *src = data;
- partial = sctx->count & 0x3f;
sctx->count += len;
- done = 0;
- src = data;
- if ((partial + len) > 63) {
- u32 temp[SHA_WORKSPACE_WORDS];
+ if (partial && ((partial + len) >= SHA1_BLOCK_SIZE)) {
+ unsigned int done = SHA1_BLOCK_SIZE - partial;
- if (partial) {
- done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
- src = sctx->buffer;
- }
+ memcpy(sctx->buffer + partial, src, done);
+ sha_transform(sctx->state, sctx->buffer, temp);
+ len -= done;
+ src += done;
+ partial = 0;
+ }
- do {
- sha_transform(sctx->state, src, temp);
- done += 64;
- src = data + done;
- } while (done + 63 < len);
+ while (len >= SHA1_BLOCK_SIZE) {
+ sha_transform(sctx->state, src, temp);
+ len -= SHA1_BLOCK_SIZE;
+ src += SHA1_BLOCK_SIZE;
+ }
+ if (src != data)
memset(temp, 0, sizeof(temp));
- partial = 0;
- }
- memcpy(sctx->buffer + partial, src, len - done);
+
+ memcpy(sctx->buffer + partial, src, len);
return 0;
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE
2011-05-26 3:11 [PATCH] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE Mandeep Singh Baines
@ 2011-05-26 3:34 ` David Miller
2011-05-26 3:52 ` Joe Perches
2011-05-26 23:20 ` [PATCH v2] " Mandeep Singh Baines
0 siblings, 2 replies; 7+ messages in thread
From: David Miller @ 2011-05-26 3:34 UTC (permalink / raw)
To: msb; +Cc: linux-kernel, herbert, linux-crypto
From: Mandeep Singh Baines <msb@chromium.org>
Date: Wed, 25 May 2011 20:11:17 -0700
> Plus some other minor cleanup.
>
> Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
The temp[] buffer is explicitly places inside the inner most
basic block so that the compiler doesn't allocate the stack
space unless that code path is taken.
Besides the use of the SHA_* macros, I think your changes
actually make the code worse.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE
2011-05-26 3:34 ` David Miller
@ 2011-05-26 3:52 ` Joe Perches
2011-05-26 23:20 ` [PATCH v2] " Mandeep Singh Baines
1 sibling, 0 replies; 7+ messages in thread
From: Joe Perches @ 2011-05-26 3:52 UTC (permalink / raw)
To: David Miller; +Cc: msb, linux-kernel, herbert, linux-crypto
On Wed, 2011-05-25 at 23:34 -0400, David Miller wrote:
> From: Mandeep Singh Baines <msb@chromium.org>
> Date: Wed, 25 May 2011 20:11:17 -0700
> > Plus some other minor cleanup.
> > Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
> The temp[] buffer is explicitly places inside the inner most
> basic block so that the compiler doesn't allocate the stack
> space unless that code path is taken.
Does any version of gcc manage to do that?
Regardless, it's still a good idea to keep
declarations in the use scope.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE
2011-05-26 3:34 ` David Miller
2011-05-26 3:52 ` Joe Perches
@ 2011-05-26 23:20 ` Mandeep Singh Baines
2011-05-31 5:22 ` Herbert Xu
1 sibling, 1 reply; 7+ messages in thread
From: Mandeep Singh Baines @ 2011-05-26 23:20 UTC (permalink / raw)
To: David Miller; +Cc: msb, linux-kernel, herbert, linux-crypto, Joe Perches
David Miller (davem@davemloft.net) wrote:
>
> The temp[] buffer is explicitly places inside the inner most
> basic block so that the compiler doesn't allocate the stack
> space unless that code path is taken.
>
Fixed in V2 (this patch). Thanks for the review.
-- >8 -- (snip)
Plus some other minor cleanup.
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: David S. Miller <davem@davemloft.net>
Cc: Joe Perches <joe@perches.com>
Cc: linux-crypto@vger.kernel.org
---
crypto/sha1_generic.c | 33 +++++++++++++++------------------
1 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..0b56719 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -40,33 +40,30 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
- unsigned int partial, done;
- const u8 *src;
+ unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
- partial = sctx->count & 0x3f;
sctx->count += len;
- done = 0;
- src = data;
- if ((partial + len) > 63) {
+ if ((partial + len) >= SHA1_BLOCK_SIZE) {
u32 temp[SHA_WORKSPACE_WORDS];
if (partial) {
- done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
- src = sctx->buffer;
- }
-
- do {
- sha_transform(sctx->state, src, temp);
- done += 64;
- src = data + done;
- } while (done + 63 < len);
+ unsigned int done = SHA1_BLOCK_SIZE - partial;
+ memcpy(sctx->buffer + partial, data, done);
+ sha_transform(sctx->state, sctx->buffer, temp);
+ len -= done;
+ data += done;
+ partial = 0;
+ }
+ while (len >= SHA1_BLOCK_SIZE) {
+ sha_transform(sctx->state, data, temp);
+ len -= SHA1_BLOCK_SIZE;
+ data += SHA1_BLOCK_SIZE;
+ }
memset(temp, 0, sizeof(temp));
- partial = 0;
}
- memcpy(sctx->buffer + partial, src, len - done);
+ memcpy(sctx->buffer + partial, data, len);
return 0;
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE
2011-05-26 23:20 ` [PATCH v2] " Mandeep Singh Baines
@ 2011-05-31 5:22 ` Herbert Xu
2011-06-23 19:02 ` [PATCH v3] crypto: sha1: " Mandeep Singh Baines
0 siblings, 1 reply; 7+ messages in thread
From: Herbert Xu @ 2011-05-31 5:22 UTC (permalink / raw)
To: Mandeep Singh Baines
Cc: David Miller, linux-kernel, linux-crypto, Joe Perches
On Thu, May 26, 2011 at 04:20:58PM -0700, Mandeep Singh Baines wrote:
> David Miller (davem@davemloft.net) wrote:
> >
> > The temp[] buffer is explicitly places inside the inner most
> > basic block so that the compiler doesn't allocate the stack
> > space unless that code path is taken.
> >
>
> Fixed in V2 (this patch). Thanks for the review.
>
> -- >8 -- (snip)
>
> Plus some other minor cleanup.
I don't really like the cleanups. In any case, mixing up the
use of SHA1_BLOCK_SIZE with cleanups increases the chance of
introducing a bug.
So please redo the patch with only the addition of SHA1_BLOCK_SIZE
and nothing else.
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] 7+ messages in thread
* [PATCH v3] crypto: sha1: use SHA1_BLOCK_SIZE
2011-05-31 5:22 ` Herbert Xu
@ 2011-06-23 19:02 ` Mandeep Singh Baines
2011-06-27 7:42 ` Herbert Xu
0 siblings, 1 reply; 7+ messages in thread
From: Mandeep Singh Baines @ 2011-06-23 19:02 UTC (permalink / raw)
To: Herbert Xu
Cc: Mandeep Singh Baines, David Miller, linux-kernel, linux-crypto,
Joe Perches
Modify sha1_update to use SHA1_BLOCK_SIZE.
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
---
crypto/sha1_generic.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..00ae60e 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -43,25 +43,26 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int partial, done;
const u8 *src;
- partial = sctx->count & 0x3f;
+ partial = sctx->count % SHA1_BLOCK_SIZE;
sctx->count += len;
done = 0;
src = data;
- if ((partial + len) > 63) {
+ if ((partial + len) >= SHA1_BLOCK_SIZE) {
u32 temp[SHA_WORKSPACE_WORDS];
if (partial) {
done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
+ memcpy(sctx->buffer + partial, data,
+ done + SHA1_BLOCK_SIZE);
src = sctx->buffer;
}
do {
sha_transform(sctx->state, src, temp);
- done += 64;
+ done += SHA1_BLOCK_SIZE;
src = data + done;
- } while (done + 63 < len);
+ } while (done + SHA1_BLOCK_SIZE <= len);
memset(temp, 0, sizeof(temp));
partial = 0;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] crypto: sha1: use SHA1_BLOCK_SIZE
2011-06-23 19:02 ` [PATCH v3] crypto: sha1: " Mandeep Singh Baines
@ 2011-06-27 7:42 ` Herbert Xu
0 siblings, 0 replies; 7+ messages in thread
From: Herbert Xu @ 2011-06-27 7:42 UTC (permalink / raw)
To: Mandeep Singh Baines
Cc: Mandeep Singh Baines, David Miller, linux-kernel, linux-crypto,
Joe Perches
On Thu, Jun 23, 2011 at 12:02:27PM -0700, Mandeep Singh Baines wrote:
> Modify sha1_update to use SHA1_BLOCK_SIZE.
>
> Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Patch applied. Thanks a lot!
--
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] 7+ messages in thread
end of thread, other threads:[~2011-06-27 7:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-26 3:11 [PATCH] crypto: sha1: modify sha1_update to use SHA1_BLOCK_SIZE Mandeep Singh Baines
2011-05-26 3:34 ` David Miller
2011-05-26 3:52 ` Joe Perches
2011-05-26 23:20 ` [PATCH v2] " Mandeep Singh Baines
2011-05-31 5:22 ` Herbert Xu
2011-06-23 19:02 ` [PATCH v3] crypto: sha1: " Mandeep Singh Baines
2011-06-27 7:42 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).