Thu Aug 23 01:55:38 2018 UTC ()

Don't call key_ismyaddr, which may sleep, in a pserialize read section

Use mutex here instead of pserialize because using mutex is simpler than
using psz+ref, which is another solution, and key_checkspidup isn't called in
any performance-sensitive paths.


(ozaki-r)

diff -r1.256 -r1.257 src/sys/netipsec/key.c

--- src/sys/netipsec/key.c 2018/07/04 19:20:25	1.256
+++ src/sys/netipsec/key.c 2018/08/23 01:55:38	1.257

 @@ -1,14 +1,14 @@
-/*	$NetBSD: key.c,v 1.256 2018/07/04 19:20:25 christos Exp $	*/
+/*	$NetBSD: key.c,v 1.257 2018/08/23 01:55:38 ozaki-r Exp $	*/
 /*	$FreeBSD: key.c,v 1.3.2.3 2004/02/14 22:23:23 bms Exp $	*/
 /*	$KAME: key.c,v 1.191 2001/06/27 10:46:49 sakane Exp $	*/
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
  * All rights reserved.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
 @@ -22,27 +22,27 @@
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
  * ARE DISCLAIMED.  IN NO EVENT SHALL THE PROJECT OR CONTRIBUTORS BE LIABLE
  * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
  * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
  * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
  * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.256 2018/07/04 19:20:25 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.257 2018/08/23 01:55:38 ozaki-r Exp $");
 /*
  * This code is referred to RFC 2367
  */
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
 #include "opt_ipsec.h"
 #include "opt_gateway.h"
 #include "opt_net_mpsafe.h"
 #endif
 #include <sys/types.h>
 @@ -3445,49 +3445,49 @@ key_getsah_ref(const struct secasindex *
 /*
  * check not to be duplicated SPI.
  * NOTE: this function is too slow due to searching all SAD.
  * OUT:
  *	NULL	: not found
  *	others	: found, pointer to a SA.
  */
 static bool
 key_checkspidup(const struct secasindex *saidx, u_int32_t spi)
+{
 	struct secashead *sah;
 	struct secasvar *sav;
 	int s;
 	/* check address family */
 	if (saidx->src.sa.sa_family != saidx->dst.sa.sa_family) {
 		IPSECLOG(LOG_DEBUG,
 		    "address family mismatched src %u, dst %u.\n",
 		    saidx->src.sa.sa_family, saidx->dst.sa.sa_family);
 		return false;
+	}
 	/* check all SAD */
-	s = pserialize_read_enter();
+	/* key_ismyaddr may sleep, so use mutex, not pserialize, here. */
-	SAHLIST_READER_FOREACH(sah) {
+	mutex_enter(&key_sad.lock);
 	SAHLIST_WRITER_FOREACH(sah) {
 		if (!key_ismyaddr((struct sockaddr *)&sah->saidx.dst))
 			continue;
 		sav = key_getsavbyspi(sah, spi);
 		if (sav != NULL) {
 			pserialize_read_exit(s);
 			KEY_SA_UNREF(&sav);
 			mutex_exit(&key_sad.lock);
 			return true;
+		}
+	}
-	pserialize_read_exit(s);
+	mutex_exit(&key_sad.lock);
 	return false;
+}
 /*
  * search SAD litmited alive SA, protocol, SPI.
  * OUT:
  *	NULL	: not found
  *	others	: found, pointer to a SA.
  */
 static struct secasvar *
 key_getsavbyspi(struct secashead *sah, u_int32_t spi)
+{