From owner-svn-src-head@FreeBSD.ORG Mon Apr 27 00:55:59 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 219DBF4; Mon, 27 Apr 2015 00:55:59 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 033FD16CE; Mon, 27 Apr 2015 00:55:59 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t3R0twBN078540; Mon, 27 Apr 2015 00:55:58 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t3R0tvEG078524; Mon, 27 Apr 2015 00:55:57 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201504270055.t3R0tvEG078524@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Mon, 27 Apr 2015 00:55:57 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r282046 - in head/sys: netinet netinet6 netipsec X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Apr 2015 00:55:59 -0000 Author: ae Date: Mon Apr 27 00:55:56 2015 New Revision: 282046 URL: https://svnweb.freebsd.org/changeset/base/282046 Log: 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 Modified: head/sys/netinet/ip_ipsec.c head/sys/netinet6/ip6_forward.c head/sys/netinet6/ip6_ipsec.c 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/netinet/ip_ipsec.c ============================================================================== --- head/sys/netinet/ip_ipsec.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netinet/ip_ipsec.c Mon Apr 27 00:55:56 2015 (r282046) @@ -199,6 +199,9 @@ ip_ipsec_output(struct mbuf **m, struct /* 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 @@ -238,9 +241,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); Modified: head/sys/netinet6/ip6_forward.c ============================================================================== --- head/sys/netinet6/ip6_forward.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netinet6/ip6_forward.c Mon Apr 27 00:55:56 2015 (r282046) @@ -248,8 +248,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. @@ -283,9 +282,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 Modified: head/sys/netinet6/ip6_ipsec.c ============================================================================== --- head/sys/netinet6/ip6_ipsec.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netinet6/ip6_ipsec.c Mon Apr 27 00:55:56 2015 (r282046) @@ -213,7 +213,9 @@ ip6_ipsec_output(struct mbuf **m, struct /* 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 @@ -253,9 +255,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); Modified: head/sys/netipsec/ipsec_output.c ============================================================================== --- head/sys/netipsec/ipsec_output.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netipsec/ipsec_output.c Mon Apr 27 00:55:56 2015 (r282046) @@ -104,6 +104,7 @@ ipsec_process_done(struct mbuf *m, struc 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")); @@ -163,6 +164,10 @@ ipsec_process_done(struct mbuf *m, struc * 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. */ @@ -170,7 +175,11 @@ ipsec_process_done(struct mbuf *m, struc #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 @@ -178,7 +187,11 @@ ipsec_process_done(struct mbuf *m, struc 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 Modified: head/sys/netipsec/xform_ah.c ============================================================================== --- head/sys/netipsec/xform_ah.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netipsec/xform_ah.c Mon Apr 27 00:55:56 2015 (r282046) @@ -1091,6 +1091,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. */ @@ -1154,16 +1155,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 = { Modified: head/sys/netipsec/xform_esp.c ============================================================================== --- head/sys/netipsec/xform_esp.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netipsec/xform_esp.c Mon Apr 27 00:55:56 2015 (r282046) @@ -888,6 +888,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. */ @@ -966,16 +967,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 = { Modified: head/sys/netipsec/xform_ipcomp.c ============================================================================== --- head/sys/netipsec/xform_ipcomp.c Mon Apr 27 00:39:57 2015 (r282045) +++ head/sys/netipsec/xform_ipcomp.c Mon Apr 27 00:55:56 2015 (r282046) @@ -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 = {