The correct way to calculate the start of the aligned part of an
unaligned buffer is:
offset = ALIGN(offset, alignmask + 1);
However, crypto_hash_walk_done() has:
offset += alignmask - 1;
offset = ALIGN(offset, alignmask + 1);
which actually skips a whole block unless offset % (alignmask + 1) == 1.
This patch fixes the problem.
Signed-off-by: Szilveszter ?rd?g <[email protected]>
---
crypto/ahash.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index f347637..db42202 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -65,7 +65,6 @@ int crypto_hash_walk_done(struct crypto_hash_walk
*walk, int err)
walk->data -= walk->offset;
if (nbytes && walk->offset & alignmask && !err) {
- walk->offset += alignmask - 1;
walk->offset = ALIGN(walk->offset, alignmask + 1);
walk->data += walk->offset;
--
1.5.5.6
On Tue, Feb 23, 2010 at 11:50:01AM +0000, Szilveszter Ordog wrote:
> The correct way to calculate the start of the aligned part of an
> unaligned buffer is:
>
> offset = ALIGN(offset, alignmask + 1);
>
> However, crypto_hash_walk_done() has:
>
> offset += alignmask - 1;
> offset = ALIGN(offset, alignmask + 1);
>
> which actually skips a whole block unless offset % (alignmask + 1) == 1.
>
> This patch fixes the problem.
>
> Signed-off-by: Szilveszter ?rd?g <[email protected]>
I think you did find a bug, but it's not what you think it is :)
When we get an unaligned buffer, we first process the bit from the
start to the first aligned address. Once we get to the aligned
address everything happens as usual.
So where this code is, we're trying to move to the next aligned
address, and as ALIGN rounds down, we need to add alignmask.
So the bug is the fact that we're adding alignmask - 1.
Were you able to reproduce this? If so please give this patch a
spin.
commit 483b84aa69382d581f272e882158b91387fa2b7a
Author: Herbert Xu <[email protected]>
Date: Tue Mar 2 22:36:33 2010 +0800
crypto: hash - Fix SG walk on unaligned addresses
When we do an SG walk on an unaligned address that is exactly 1
modulo the alignment, we end up hashing some of the data twice.
Reported-by: Szilveszter Ordog <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 33a4ff4..b52eb6d 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -78,7 +78,7 @@ int crypto_hash_walk_done(struct crypto_hash_walk *walk, int err)
walk->data -= walk->offset;
if (nbytes && walk->offset & alignmask && !err) {
- walk->offset += alignmask - 1;
+ walk->offset += alignmask;
walk->offset = ALIGN(walk->offset, alignmask + 1);
walk->data += walk->offset;
Thanks,
--
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
> When we get an unaligned buffer, we first process the bit from the
> start to the first aligned address. ?Once we get to the aligned
> address everything happens as usual.
>
> So where this code is, we're trying to move to the next aligned
> address, and as ALIGN rounds down, we need to add alignmask.
> So the bug is the fact that we're adding alignmask - 1.
No, ALIGN rounds *up* (as it should). See:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=include/linux/kernel.h#l40
Best regards,
Szilveszter
Szilveszter Ordog <[email protected]> wrote:
>> When we get an unaligned buffer, we first process the bit from the
>> start to the first aligned address. ?Once we get to the aligned
>> address everything happens as usual.
>>
>> So where this code is, we're trying to move to the next aligned
>> address, and as ALIGN rounds down, we need to add alignmask.
>> So the bug is the fact that we're adding alignmask - 1.
>
> No, ALIGN rounds *up* (as it should). See:
Ah, of course it does. I've applied your patch. Thanks a lot!
--
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
> I've applied your patch.
Great! Thank you!
Best regards,
Szilveszter