[PATCH 0/2] AH fixes for asynchronous hash algorithms.

Here are two fixes for AH when using an asynchronous hmac driver.  Both
are -stable candidates as these problems appear to have been present
since AH was converted to use ahash way back in 2.6.33.

These code paths are not exercised when using the default software hash
implementations which do not use the ahash callbacks, but the issues can be
reproduced by using cryptd to create an asynchronous hash algorithm for
testing.

This driver could probably do with some cleanups to reduce the code
duplication (and thus test coverage) between the asynchronous callbacks
and synchronous code paths, which should help avoid these kind of
problems in the future.  These code paths apparently do not see a
lot of testing.  But that's for a later patch series.

Nick Bowler (2):
  ah: Correctly pass error codes in ahash output callback.
  ah: Read nexthdr value before overwriting it in ahash input callback.

 net/ipv4/ah4.c |    6 ++----
 net/ipv6/ah6.c |    6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

-- 
1.7.3.4

The AH4/6 ahash output callbacks pass nexthdr to xfrm_output_resume
instead of the error code.  This appears to be a copy+paste error from
the input case, where nexthdr is expected.  This causes the driver to
continuously add AH headers to the datagram until either an allocation
fails and the packet is dropped or the ahash driver hits a synchronous
fallback and the resulting monstrosity is transmitted.

Correct this issue by simply passing the error code unadulterated.

Signed-off-by: Nick Bowler <redacted>
---
 net/ipv4/ah4.c |    2 --
 net/ipv6/ah6.c |    2 --
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index c1f4154..33ca186 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c

@@ -136,8 +136,6 @@ static void ah_output_done(struct crypto_async_request *base, int err)
 		memcpy(top_iph+1, iph+1, top_iph->ihl*4 - sizeof(struct iphdr));
 	}
 
-	err = ah->nexthdr;
-
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_output_resume(skb, err);
 }

diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 2195ae6..ede4d9d 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c

@@ -324,8 +324,6 @@ static void ah6_output_done(struct crypto_async_request *base, int err)
 #endif
 	}
 
-	err = ah->nexthdr;
-
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_output_resume(skb, err);
 }

-- 
1.7.3.4

The AH4/6 ahash input callbacks read out the nexthdr field from the AH
header *after* they overwrite that header.  This is obviously not going
to end well.  Fix it up.

Signed-off-by: Nick Bowler <redacted>
---
 net/ipv4/ah4.c |    4 ++--
 net/ipv6/ah6.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 33ca186..c7056b2 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c

@@ -262,12 +262,12 @@ static void ah_input_done(struct crypto_async_request *base, int err)
 	if (err)
 		goto out;
 
+	err = ah->nexthdr;
+
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, ihl);
 	__skb_pull(skb, ah_hlen + ihl);
 	skb_set_transport_header(skb, -ihl);
-
-	err = ah->nexthdr;
 out:
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_input_resume(skb, err);

diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index ede4d9d..7a33aaa 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c

@@ -464,12 +464,12 @@ static void ah6_input_done(struct crypto_async_request *base, int err)
 	if (err)
 		goto out;
 
+	err = ah->nexthdr;
+
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, hdr_len);
 	__skb_pull(skb, ah_hlen + hdr_len);
 	skb_set_transport_header(skb, -hdr_len);
-
-	err = ah->nexthdr;
 out:
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_input_resume(skb, err);

-- 
1.7.3.4

From: Nick Bowler <redacted>
Date: Tue,  8 Nov 2011 17:12:43 -0500

Here are two fixes for AH when using an asynchronous hmac driver.  Both
are -stable candidates as these problems appear to have been present
since AH was converted to use ahash way back in 2.6.33.

These code paths are not exercised when using the default software hash
implementations which do not use the ahash callbacks, but the issues can be
reproduced by using cryptd to create an asynchronous hash algorithm for
testing.

This driver could probably do with some cleanups to reduce the code
duplication (and thus test coverage) between the asynchronous callbacks
and synchronous code paths, which should help avoid these kind of
problems in the future.  These code paths apparently do not see a
lot of testing.  But that's for a later patch series.

Nick Bowler (2):
  ah: Correctly pass error codes in ahash output callback.
  ah: Read nexthdr value before overwriting it in ahash input callback.

Thanks a lot for these bug fixes Nick, both applied.

Also queued up for -stable.