summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuiz Otavio O Souza <luiz@netgate.com>2015-09-15 15:27:49 -0500
committerLuiz Otavio O Souza <luiz@netgate.com>2015-10-20 12:02:15 -0500
commit5696701b2e19f4a198340ada5b4ba73fa5c6c9f9 (patch)
tree93cc9ec4253fb4f506b392065fab2214e23b2e51
parentc10a06fdafd86efa4c2c4c262870f0d5276dd625 (diff)
downloadFreeBSD-src-5696701b2e19f4a198340ada5b4ba73fa5c6c9f9.zip
FreeBSD-src-5696701b2e19f4a198340ada5b4ba73fa5c6c9f9.tar.gz
MFC r282046:
Fix possible use after free due to security policy deletion. When we are passing mbuf to IPSec processing via ipsec[46]_process_packet(), we hold one reference to security policy and release it just after return from this function. But IPSec processing can be deffered and when we release reference to security policy after ipsec[46]_process_packet(), user can delete this security policy from SPDB. And when IPSec processing will be done, xform's callback function will do access to already freed memory. To fix this move KEY_FREESP() into callback function. Now IPSec code will release reference to SP after processing will be finished. Differential Revision: https://reviews.freebsd.org/D2324 No objections from: #network Sponsored by: Yandex LLC TAG: IPSEC-HEAD Issue: #4841
-rw-r--r--sys/netinet/ip_ipsec.c7
-rw-r--r--sys/netinet6/ip6_forward.c9
-rw-r--r--sys/netinet6/ip6_ipsec.c8
-rw-r--r--sys/netipsec/ipsec_output.c17
-rw-r--r--sys/netipsec/xform_ah.c7
-rw-r--r--sys/netipsec/xform_esp.c7
-rw-r--r--sys/netipsec/xform_ipcomp.c7
7 files changed, 42 insertions, 20 deletions
diff --git a/sys/netinet/ip_ipsec.c b/sys/netinet/ip_ipsec.c
index 9c3d631..6fc8e80 100644
--- a/sys/netinet/ip_ipsec.c
+++ b/sys/netinet/ip_ipsec.c
@@ -198,6 +198,9 @@ ip_ipsec_output(struct mbuf **m, struct inpcb *inp, int *error)
/* NB: callee frees mbuf */
*error = ipsec4_process_packet(*m, sp->req);
+ /* Release SP if an error occured */
+ if (*error != 0)
+ KEY_FREESP(&sp);
if (*error == EJUSTRETURN) {
/*
* We had a SP with a level of 'use' and no SA. We
@@ -237,9 +240,7 @@ done:
KEY_FREESP(&sp);
return 0;
reinjected:
- if (sp != NULL)
- KEY_FREESP(&sp);
- return -1;
+ return (-1);
bad:
if (sp != NULL)
KEY_FREESP(&sp);
diff --git a/sys/netinet6/ip6_forward.c b/sys/netinet6/ip6_forward.c
index 9f21150..053864a 100644
--- a/sys/netinet6/ip6_forward.c
+++ b/sys/netinet6/ip6_forward.c
@@ -250,8 +250,7 @@ ip6_forward(struct mbuf *m, int srcrt)
/*
* when the kernel forwards a packet, it is not proper to apply
- * IPsec transport mode to the packet is not proper. this check
- * avoid from this.
+ * IPsec transport mode to the packet. This check avoid from this.
* at present, if there is even a transport mode SA request in the
* security policy, the kernel does not apply IPsec to the packet.
* this check is not enough because the following case is valid.
@@ -285,9 +284,9 @@ ip6_forward(struct mbuf *m, int srcrt)
* ipsec6_proces_packet will send the packet using ip6_output
*/
error = ipsec6_process_packet(m, sp->req);
-
- KEY_FREESP(&sp);
-
+ /* Release SP if an error occured */
+ if (error != 0)
+ KEY_FREESP(&sp);
if (error == EJUSTRETURN) {
/*
* We had a SP with a level of 'use' and no SA. We
diff --git a/sys/netinet6/ip6_ipsec.c b/sys/netinet6/ip6_ipsec.c
index 66459cf..ac49275 100644
--- a/sys/netinet6/ip6_ipsec.c
+++ b/sys/netinet6/ip6_ipsec.c
@@ -212,7 +212,9 @@ ip6_ipsec_output(struct mbuf **m, struct inpcb *inp, int *error)
/* NB: callee frees mbuf */
*error = ipsec6_process_packet(*m, sp->req);
-
+ /* Release SP if an error occured */
+ if (*error != 0)
+ KEY_FREESP(&sp);
if (*error == EJUSTRETURN) {
/*
* We had a SP with a level of 'use' and no SA. We
@@ -252,9 +254,7 @@ done:
KEY_FREESP(&sp);
return 0;
reinjected:
- if (sp != NULL)
- KEY_FREESP(&sp);
- return -1;
+ return (-1);
bad:
if (sp != NULL)
KEY_FREESP(&sp);
diff --git a/sys/netipsec/ipsec_output.c b/sys/netipsec/ipsec_output.c
index a201fb6..288770c 100644
--- a/sys/netipsec/ipsec_output.c
+++ b/sys/netipsec/ipsec_output.c
@@ -103,6 +103,7 @@ ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr)
IPSEC_ASSERT(m != NULL, ("null mbuf"));
IPSEC_ASSERT(isr != NULL, ("null ISR"));
+ IPSEC_ASSERT(isr->sp != NULL, ("NULL isr->sp"));
sav = isr->sav;
IPSEC_ASSERT(sav != NULL, ("null SA"));
IPSEC_ASSERT(sav->sah != NULL, ("null SAH"));
@@ -162,6 +163,10 @@ ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr)
* If this is a problem we'll need to introduce a queue
* to set the packet on so we can unwind the stack before
* doing further processing.
+ *
+ * If ipsec[46]_process_packet() will successfully queue
+ * the request, we need to take additional reference to SP,
+ * because xform callback will release reference.
*/
if (isr->next) {
/* XXX-BZ currently only support same AF bundles. */
@@ -169,7 +174,11 @@ ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr)
#ifdef INET
case AF_INET:
IPSECSTAT_INC(ips_out_bundlesa);
- return ipsec4_process_packet(m, isr->next);
+ key_addref(isr->sp);
+ error = ipsec4_process_packet(m, isr->next);
+ if (error != 0)
+ KEY_FREESP(&isr->sp);
+ return (error);
/* NOTREACHED */
#endif
#ifdef notyet
@@ -177,7 +186,11 @@ ipsec_process_done(struct mbuf *m, struct ipsecrequest *isr)
case AF_INET6:
/* XXX */
IPSEC6STAT_INC(ips_out_bundlesa);
- return ipsec6_process_packet(m, isr->next);
+ key_addref(isr->sp);
+ error = ipsec6_process_packet(m, isr->next);
+ if (error != 0)
+ KEY_FREESP(&isr->sp);
+ return (error);
/* NOTREACHED */
#endif /* INET6 */
#endif
diff --git a/sys/netipsec/xform_ah.c b/sys/netipsec/xform_ah.c
index 1c5949c..5274ea2 100644
--- a/sys/netipsec/xform_ah.c
+++ b/sys/netipsec/xform_ah.c
@@ -1089,6 +1089,7 @@ ah_output_cb(struct cryptop *crp)
m = (struct mbuf *) crp->crp_buf;
isr = tc->tc_isr;
+ IPSEC_ASSERT(isr->sp != NULL, ("NULL isr->sp"));
IPSECREQUEST_LOCK(isr);
sav = tc->tc_sav;
/* With the isr lock released SA pointer can be updated. */
@@ -1152,16 +1153,18 @@ ah_output_cb(struct cryptop *crp)
error = ipsec_process_done(m, isr);
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
- return error;
+ KEY_FREESP(&isr->sp);
+ return (error);
bad:
if (sav)
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
+ KEY_FREESP(&isr->sp);
if (m)
m_freem(m);
free(tc, M_XDATA);
crypto_freereq(crp);
- return error;
+ return (error);
}
static struct xformsw ah_xformsw = {
diff --git a/sys/netipsec/xform_esp.c b/sys/netipsec/xform_esp.c
index 2e8cded..b5a22ba 100644
--- a/sys/netipsec/xform_esp.c
+++ b/sys/netipsec/xform_esp.c
@@ -886,6 +886,7 @@ esp_output_cb(struct cryptop *crp)
m = (struct mbuf *) crp->crp_buf;
isr = tc->tc_isr;
+ IPSEC_ASSERT(isr->sp != NULL, ("NULL isr->sp"));
IPSECREQUEST_LOCK(isr);
sav = tc->tc_sav;
/* With the isr lock released SA pointer can be updated. */
@@ -964,16 +965,18 @@ esp_output_cb(struct cryptop *crp)
error = ipsec_process_done(m, isr);
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
- return error;
+ KEY_FREESP(&isr->sp);
+ return (error);
bad:
if (sav)
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
+ KEY_FREESP(&isr->sp);
if (m)
m_freem(m);
free(tc, M_XDATA);
crypto_freereq(crp);
- return error;
+ return (error);
}
static struct xformsw esp_xformsw = {
diff --git a/sys/netipsec/xform_ipcomp.c b/sys/netipsec/xform_ipcomp.c
index 1519f15..a5d1e57 100644
--- a/sys/netipsec/xform_ipcomp.c
+++ b/sys/netipsec/xform_ipcomp.c
@@ -492,6 +492,7 @@ ipcomp_output_cb(struct cryptop *crp)
skip = tc->tc_skip;
isr = tc->tc_isr;
+ IPSEC_ASSERT(isr->sp != NULL, ("NULL isr->sp"));
IPSECREQUEST_LOCK(isr);
sav = tc->tc_sav;
/* With the isr lock released SA pointer can be updated. */
@@ -606,16 +607,18 @@ ipcomp_output_cb(struct cryptop *crp)
error = ipsec_process_done(m, isr);
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
- return error;
+ KEY_FREESP(&isr->sp);
+ return (error);
bad:
if (sav)
KEY_FREESAV(&sav);
IPSECREQUEST_UNLOCK(isr);
+ KEY_FREESP(&isr->sp);
if (m)
m_freem(m);
free(tc, M_XDATA);
crypto_freereq(crp);
- return error;
+ return (error);
}
static struct xformsw ipcomp_xformsw = {
OpenPOWER on IntegriCloud