From owner-svn-src-all@freebsd.org Tue May 23 09:32:28 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3604AD79803; Tue, 23 May 2017 09:32:28 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::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 11767155E; Tue, 23 May 2017 09:32:27 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v4N9WRki032054; Tue, 23 May 2017 09:32:27 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v4N9WQtT032050; Tue, 23 May 2017 09:32:26 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201705230932.v4N9WQtT032050@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Tue, 23 May 2017 09:32:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r318738 - head/sys/netipsec X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 May 2017 09:32:28 -0000 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); }