Re: [PATCH v2] crypto: hash - Fix page length clamping in hash walk

2016-05-04 Thread Sowmini Varadhan 
On (05/04/16 12:08), Steffen Klassert wrote:
> > > Sowmini, could you please doublecheck with your test setup?
> > 
> > Don't bother, my patch was crap.  Here's one that's more likely
> > to work:
> 
> This one is much better, works here :)

The patch seems to work, but the caveat is that the original
bug (iperf segv) was not always reproducible on demand - it depended
on memory and other config. 

But looks like Steffen has a reliable way to reproduce the original
problem, so lets go with this patch for now.

--Sowmini

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] crypto: hash - Fix page length clamping in hash walk

2016-05-04 Thread Steffen Klassert 
On Wed, May 04, 2016 at 05:52:56PM +0800, Herbert Xu wrote:
> On Wed, May 04, 2016 at 11:34:20AM +0200, Steffen Klassert wrote:
> > 
> > Hmm, the 'sleeping while atomic' because of not unmapping
> > the page goes away, but now I see a lot of IPsec ICV fails
> > on the receive side. I'll try to find out what's going on.
> > 
> > Sowmini, could you please doublecheck with your test setup?
> 
> Don't bother, my patch was crap.  Here's one that's more likely
> to work:

This one is much better, works here :)

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] crypto: hash - Fix page length clamping in hash walk

2016-05-04 Thread Herbert Xu 
On Wed, May 04, 2016 at 11:34:20AM +0200, Steffen Klassert wrote:
> 
> Hmm, the 'sleeping while atomic' because of not unmapping
> the page goes away, but now I see a lot of IPsec ICV fails
> on the receive side. I'll try to find out what's going on.
> 
> Sowmini, could you please doublecheck with your test setup?

Don't bother, my patch was crap.  Here's one that's more likely
to work:

---8<---
The crypto hash walk code is broken when supplied with an offset
greater than or equal to PAGE_SIZE.  This patch fixes it by adjusting
walk->pg and walk->offset when this happens.

Cc: 
Reported-by: Steffen Klassert 
Signed-off-by: Herbert Xu 

diff --git a/crypto/ahash.c b/crypto/ahash.c
index 5fc1f17..3887a98 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -69,8 +69,9 @@ static int hash_walk_new_entry(struct crypto_hash_walk *walk)
struct scatterlist *sg;
 
sg = walk->sg;
-   walk->pg = sg_page(sg);
walk->offset = sg->offset;
+   walk->pg = sg_page(walk->sg) + (walk->offset >> PAGE_SHIFT);
+   walk->offset = offset_in_page(walk->offset);
walk->entrylen = sg->length;
 
if (walk->entrylen > walk->total)
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto: hash - Fix page length clamping in hash walk

2016-05-04 Thread Steffen Klassert 
On Tue, May 03, 2016 at 05:55:31PM +0800, Herbert Xu wrote:
> On Thu, Apr 28, 2016 at 10:27:43AM +0200, Steffen Klassert wrote:
> >
> > The problem was that if offset (in a superpage) equals
> > PAGE_SIZE in hash_walk_next(), nbytes becomes zero. So
> > we map the page, but we don't hash and unmap because we
> > exit the loop in shash_ahash_update() in this case.
> 
> I see.  Does this patch help?

Hmm, the 'sleeping while atomic' because of not unmapping
the page goes away, but now I see a lot of IPsec ICV fails
on the receive side. I'll try to find out what's going on.

Sowmini, could you please doublecheck with your test setup?

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

crypto: hash - Fix page length clamping in hash walk

2016-05-03 Thread Herbert Xu 
On Thu, Apr 28, 2016 at 10:27:43AM +0200, Steffen Klassert wrote:
>
> The problem was that if offset (in a superpage) equals
> PAGE_SIZE in hash_walk_next(), nbytes becomes zero. So
> we map the page, but we don't hash and unmap because we
> exit the loop in shash_ahash_update() in this case.

I see.  Does this patch help?

---8<---
The length clamping in the crypto hash walk code is broken if
supplied with an offset greater than or equal to PAGE_SIZE.  This
patch fixes it by borrowing the code from scatterwalk.

Cc: 
Reported-by: Steffen Klassert 
Signed-off-by: Herbert Xu 

diff --git a/crypto/ahash.c b/crypto/ahash.c
index 5fc1f17..2d6c4f1 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -44,8 +44,8 @@ static int hash_walk_next(struct crypto_hash_walk *walk)
 {
unsigned int alignmask = walk->alignmask;
unsigned int offset = walk->offset;
-   unsigned int nbytes = min(walk->entrylen,
- ((unsigned int)(PAGE_SIZE)) - offset);
+   unsigned int nbytes = min_t(unsigned int, walk->entrylen,
+   offset_in_page(~offset) + 1);
 
if (walk->flags & CRYPTO_ALG_ASYNC)
walk->data = kmap(walk->pg);
@@ -91,8 +91,8 @@ int crypto_hash_walk_done(struct crypto_hash_walk *walk, int 
err)
walk->offset = ALIGN(walk->offset, alignmask + 1);
walk->data += walk->offset;
 
-   nbytes = min(nbytes,
-((unsigned int)(PAGE_SIZE)) - walk->offset);
+   nbytes = min_t(unsigned int, nbytes,
+  offset_in_page(~walk->offset) + 1);
walk->entrylen -= nbytes;
 
return nbytes;
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html