From 3aee70991d8901dedb15c4a97d011be7aa679b33 Mon Sep 17 00:00:00 2001
From: "Andrey V. Elsukov" <ae@FreeBSD.org>
Date: Tue, 23 May 2017 09:32:26 +0000
Subject: [PATCH] Fix possible double releasing for SA and SP references.

There are two possible ways how crypto callback are called: directly from
caller and deffered from crypto thread.

For outbound packets the direct call chain is the following:
 IPSEC_OUTPUT() method -> ipsec[46]_common_output() ->
 -> ipsec[46]_perform_request() -> xform_output() ->
 -> crypto_dispatch() -> crypto_invoke() -> crypto_done() ->
 -> xform_output_cb() -> ipsec_process_done() -> ip[6]_output().

The SA and SP references are held while crypto processing is not finished.
The error handling code wrongly expected that crypto callback always called
from the crypto thread context, and it did references releasing in
xform_output_cb(). But when the crypto callback called directly, in case of
error the error handling code in ipsec[46]_perform_request() also did
references releasing.

To fix this, remove error handling from ipsec[46]_perform_request() and do it
in xform_output() before crypto_dispatch().

MFC after:	10 days
---
 sys/netipsec/ipsec_output.c | 8 --------
 sys/netipsec/xform_ah.c     | 2 ++
 sys/netipsec/xform_esp.c    | 2 ++
 sys/netipsec/xform_ipcomp.c | 2 ++
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/sys/netipsec/ipsec_output.c b/sys/netipsec/ipsec_output.c
index 392008e9197..80b6ab8300b 100644
--- a/sys/netipsec/ipsec_output.c
+++ b/sys/netipsec/ipsec_output.c
@@ -273,10 +273,6 @@ ipsec4_perform_request(struct mbuf *m, struct secpolicy *sp, u_int idx)
 		goto bad;
 	}
 	error = (*sav->tdb_xform->xf_output)(m, sp, sav, idx, i, off);
-	if (error != 0) {
-		key_freesav(&sav);
-		key_freesp(&sp);
-	}
 	return (error);
 bad:
 	IPSECSTAT_INC(ips_out_inval);
@@ -581,10 +577,6 @@ ipsec6_perform_request(struct mbuf *m, struct secpolicy *sp, u_int idx)
 		goto bad;
 	}
 	error = (*sav->tdb_xform->xf_output)(m, sp, sav, idx, i, off);
-	if (error != 0) {
-		key_freesav(&sav);
-		key_freesp(&sp);
-	}
 	return (error);
 bad:
 	IPSEC6STAT_INC(ips_out_inval);
diff --git a/sys/netipsec/xform_ah.c b/sys/netipsec/xform_ah.c
index 521ce3447d1..0bcab46373c 100644
--- a/sys/netipsec/xform_ah.c
+++ b/sys/netipsec/xform_ah.c
@@ -1049,6 +1049,8 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }
 
diff --git a/sys/netipsec/xform_esp.c b/sys/netipsec/xform_esp.c
index bf6fc9cb197..7eda6450bcb 100644
--- a/sys/netipsec/xform_esp.c
+++ b/sys/netipsec/xform_esp.c
@@ -861,6 +861,8 @@ esp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }
 /*
diff --git a/sys/netipsec/xform_ipcomp.c b/sys/netipsec/xform_ipcomp.c
index a18c340ddf5..061937d7022 100644
--- a/sys/netipsec/xform_ipcomp.c
+++ b/sys/netipsec/xform_ipcomp.c
@@ -510,6 +510,8 @@ ipcomp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }