On Sun, Mar 25, 2018 at 11:49:58PM +0800, Eli Cooper wrote:
> When supplied with an offset equal to PAGE_SIZE, the hash walk code returns
> zero, resulting in early termination and, in the observed scenario, memory
> corruption. This patch fixes it by clamping nbytes only when walk->offset
> is not at the PAGE_SIZE boundary.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Eli Cooper <elicooper@xxxxxxx>
> ---
> crypto/ahash.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/crypto/ahash.c b/crypto/ahash.c
> index 3a35d67de7d9..03cbe04c53b1 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -46,8 +46,11 @@ 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 pagelen, nbytes = walk->entrylen;
> +
> + pagelen = ((unsigned int)(PAGE_SIZE)) - offset;
> + if (pagelen)
> + nbytes = min(nbytes, pagelen);
hash_walk_next is only called from two places, neither of which
can produce an offset which is >= PAGE_SIZE.
> @@ -94,8 +97,9 @@ 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);
> + pagelen = ((unsigned int)(PAGE_SIZE)) - walk->offset;
> + if (pagelen)
> + nbytes = min(nbytes, pagelen);
> walk->entrylen -= nbytes;
Thanks, this bug is real. However, the fix is still wrong. What
happens is that we have just done the unaligned bit in the SG list
and now the page has been consumed. We should not return directly
in this case but move onto the next page.
---8<--
When we have an unaligned SG list entry where there is no leftover
aligned data, the hash walk code will incorrectly return zero as if
the entire SG list has been processed.
This patch fixes it by moving onto the next page instead.
Reported-by: Eli Cooper <elicooper@xxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
diff --git a/crypto/ahash.c b/crypto/ahash.c
index f732dd9..a64c143 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -92,13 +92,14 @@ int crypto_hash_walk_done(struct crypto_hash_walk *walk, int err)
if (nbytes && walk->offset & alignmask && !err) {
walk->offset = ALIGN(walk->offset, alignmask + 1);
- walk->data += walk->offset;
-
nbytes = min(nbytes,
((unsigned int)(PAGE_SIZE)) - walk->offset);
walk->entrylen -= nbytes;
- return nbytes;
+ if (nbytes) {
+ walk->data += walk->offset;
+ return nbytes;
+ }
}
if (walk->flags & CRYPTO_ALG_ASYNC)
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
