svn commit: r318738 - head/sys/netipsec

[Andrey V. Elsukov]

Author: ae
Date: Tue May 23 09:32:26 2017
New Revision: 318738
URL: https://svnweb.freebsd.org/changeset/base/318738

Log:
  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

Modified:
  head/sys/netipsec/ipsec_output.c
  head/sys/netipsec/xform_ah.c
  head/sys/netipsec/xform_esp.c
  head/sys/netipsec/xform_ipcomp.c

Modified: head/sys/netipsec/ipsec_output.c
==============================================================================
--- head/sys/netipsec/ipsec_output.c	Tue May 23 09:30:42 2017	(r318737)
+++ head/sys/netipsec/ipsec_output.c	Tue May 23 09:32:26 2017	(r318738)
@@ -273,10 +273,6 @@ ipsec4_perform_request(struct mbuf *m, s
 		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, s
 		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);

Modified: head/sys/netipsec/xform_ah.c
==============================================================================
--- head/sys/netipsec/xform_ah.c	Tue May 23 09:30:42 2017	(r318737)
+++ head/sys/netipsec/xform_ah.c	Tue May 23 09:32:26 2017	(r318738)
@@ -1049,6 +1049,8 @@ ah_output(struct mbuf *m, struct secpoli
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }
 

Modified: head/sys/netipsec/xform_esp.c
==============================================================================
--- head/sys/netipsec/xform_esp.c	Tue May 23 09:30:42 2017	(r318737)
+++ head/sys/netipsec/xform_esp.c	Tue May 23 09:32:26 2017	(r318738)
@@ -861,6 +861,8 @@ esp_output(struct mbuf *m, struct secpol
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }
 /*

Modified: head/sys/netipsec/xform_ipcomp.c
==============================================================================
--- head/sys/netipsec/xform_ipcomp.c	Tue May 23 09:30:42 2017	(r318737)
+++ head/sys/netipsec/xform_ipcomp.c	Tue May 23 09:32:26 2017	(r318738)
@@ -510,6 +510,8 @@ ipcomp_output(struct mbuf *m, struct sec
 bad:
 	if (m)
 		m_freem(m);
+	key_freesav(&sav);
+	key_freesp(&sp);
 	return (error);
 }