15146 sadb_unlinkassoc() isn't idempotent

Reviewed by: Jason King <jason.brian.king+illumos@gmail.com>
Reviewed by: Robert Mustacchi <rm@fingolfin.org>
Approved by: Garrett D'Amore <garrett@damore.org>

8 files changed, 80 insertions, 7 deletions

diff --git a/usr/src/pkg/manifests/system-test-ostest.p5m b/usr/src/pkg/manifests/system-test-ostest.p5m
index 70fb4ea943..e1530adf7c 100644
--- a/usr/src/pkg/manifests/system-test-ostest.p5m
+++ b/usr/src/pkg/manifests/system-test-ostest.p5m
@@ -16,6 +16,7 @@
 # Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
 # Copyright 2021 Tintri by DDN, Inc. All rights reserved.
 # Copyright 2022 Oxide Computer Company
+# Copyright 2022 MNX Cloud, Inc.
 #
 
 set name=pkg.fmri value=pkg:/system/test/ostest@$(PKGVERS)
@@ -90,6 +91,7 @@ file path=opt/os-tests/tests/libtopo/digraph-test-in.xml mode=0444
 file path=opt/os-tests/tests/odirectory.32 mode=0555
 file path=opt/os-tests/tests/odirectory.64 mode=0555
 dir  path=opt/os-tests/tests/pf_key
+file path=opt/os-tests/tests/pf_key/15146 mode=0555
 file path=opt/os-tests/tests/pf_key/acquire-compare mode=0555
 file path=opt/os-tests/tests/pf_key/acquire-spray mode=0555
 file path=opt/os-tests/tests/pf_key/eacq-enabler mode=0555
diff --git a/usr/src/test/os-tests/runfiles/default.run b/usr/src/test/os-tests/runfiles/default.run
index 7be2dda318..7a4189cf48 100644
--- a/usr/src/test/os-tests/runfiles/default.run
+++ b/usr/src/test/os-tests/runfiles/default.run
@@ -15,6 +15,7 @@
 # Copyright 2021 OmniOS Community Edition (OmniOSce) Association.
 # Copyright 2021 Tintri by DDN, Inc. All rights reserved.
 # Copyright 2022 Oxide Computer Company
+# Copyright 2022 MNX Cloud, Inc.
 #
 
 [DEFAULT]
@@ -82,7 +83,7 @@ tests = ['fchmodat.32', 'fchmodat.64', 'open.32', 'open.64']
 [/opt/os-tests/tests/pf_key]
 user = root
 timeout = 180
-tests = ['acquire-compare', 'kmc-update']
+tests = ['acquire-compare', 'kmc-update', '15146']
 
 [/opt/os-tests/tests/ddi_ufm]
 user = root
diff --git a/usr/src/test/os-tests/tests/pf_key/15146.sh b/usr/src/test/os-tests/tests/pf_key/15146.sh
new file mode 100644
index 0000000000..e4b5558bbc
--- /dev/null
+++ b/usr/src/test/os-tests/tests/pf_key/15146.sh
@@ -0,0 +1,64 @@
+#!/usr/bin/ksh
+
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2022 MNX Cloud, Inc.
+#
+
+# Clear and load SADB, logs.
+echo "Clearing and loading SADB"
+/usr/sbin/ipseckey flush || echo "PROBLEM: ipseckey flush failed" > /dev/stderr
+echo "add esp spi 0x2112 dst 127.0.0.1 encralg aes encrkey 1234567890abcdef1234567890abcdef" | /usr/sbin/ipseckey
+/usr/sbin/ipseckey dump || echo "PROBLEM: ipseckey dump failed" > /dev/stderr
+/bin/rm -f /tmp/15146-$$-del-*
+
+# Launch DTrace trap
+# I hope .5sec is enough chill()
+/usr/sbin/dtrace -wn 'sadb_delget_sa:entry { self->trace = 1; } get_ipsa_pair:return /self->trace == 1/ { if (arg1 == 0) chill(500000000); self->trace = 0; exit(0); }' &
+dtracepid=$!
+
+# sleep for 20sec to give DTrace time, and as a starting pistol...
+/usr/bin/sleep 20 &
+pistol=$!
+
+for a in 0 1 2 3 4 5 6 7 8 9; do
+        ( pwait $pistol ; \
+                /usr/sbin/ipseckey delete esp spi 0x2112 dst 127.0.0.1 \
+                2>&1 > /tmp/15146-$$-del-$a ) &
+done
+
+# All background jobs will finish; if they don't, let the test hang, which
+# clearly indicates a problem with IPsec or DTrace.
+wait
+
+# If we reach here we haven't panicked the kernel per illumos#15146.
+# Only way otherwise we "fail" is by not seeing the race.
+
+# Check that we did delete the SA...
+/usr/sbin/ipseckey get esp spi 0x2112 dst 127.0.0.1 2>&1 > /dev/null
+if [[ $? == 0 ]]; then
+	echo "10 delete processes didn't delete ESP(spi=0x2112, dst=127.0.0.1)" \
+		> /dev/stderr
+	exit 1
+fi
+
+# See that more than one of the above processes successfully peformed DELETE.
+count=$( grep Fatal /tmp/15146-$$-del-* | wc -l )
+if [[ $count > 8 ]]; then
+	echo "Only 1 or 0 ipseckey delete processes succeeded." > /dev/stderr
+	exit 1
+fi
+
+/bin/rm -f /tmp/15146-$$-del-*
+echo "15146 appears to not affect this kernel. Good."
+exit 0
diff --git a/usr/src/test/os-tests/tests/pf_key/Makefile b/usr/src/test/os-tests/tests/pf_key/Makefile
index 34a17706b1..2ac59ff82a 100644
--- a/usr/src/test/os-tests/tests/pf_key/Makefile
+++ b/usr/src/test/os-tests/tests/pf_key/Makefile
@@ -11,13 +11,15 @@
 
 #
 # Copyright 2017 Joyent, Inc.
+# Copyright 2022 MNX Cloud, Inc.
 #
 
 include $(SRC)/cmd/Makefile.cmd
 include $(SRC)/test/Makefile.com
 
 # Shell scripts...
-PROG =	acquire-spray \
+PROG =	15146 \
+	acquire-spray \
 	acquire-compare \
 	kmc-update
 
diff --git a/usr/src/test/os-tests/tests/pf_key/acquire-compare.sh b/usr/src/test/os-tests/tests/pf_key/acquire-compare.sh
index f9b0c03e48..427b8a3459 100644
--- a/usr/src/test/os-tests/tests/pf_key/acquire-compare.sh
+++ b/usr/src/test/os-tests/tests/pf_key/acquire-compare.sh
@@ -25,7 +25,7 @@ timeout_cmd() {
     sleep 2
 }
 
-if [[ `id -u` -ne 0 ]]; then
+if [[ `id -u` != 0 ]]; then
     echo "Error: need to be root or have effective UID of root." >&2
     exit 255
 fi
diff --git a/usr/src/test/os-tests/tests/pf_key/acquire-spray.sh b/usr/src/test/os-tests/tests/pf_key/acquire-spray.sh
index cae879b4ec..f91cac2c5f 100644
--- a/usr/src/test/os-tests/tests/pf_key/acquire-spray.sh
+++ b/usr/src/test/os-tests/tests/pf_key/acquire-spray.sh
@@ -23,7 +23,7 @@
 # therefore not reliable under this kind of load.
 #
 
-if [ `id -u` -ne 0 ]; then
+if [ `id -u` != 0 ]; then
 	echo "Need to be root or have effective UID of root."
 	exit 255
 fi
@@ -99,7 +99,7 @@ svcadm restart ipsec/policy
 i=1
 while [ $i -le 254 ]; do
 	c=$(grep -c "^DST: AF_INET: port 0, 10\.21\.12\.$i\." $MONITOR_LOG)
-	if [[ "$c" -ne 2 ]]; then
+	if [[ "$c" != 2 ]]; then
 		echo "One or more log entries missing for 10.21.12.$i" >&2
 		exit 1
 	fi
diff --git a/usr/src/test/os-tests/tests/pf_key/kmc-update.sh b/usr/src/test/os-tests/tests/pf_key/kmc-update.sh
index 30cd6f988c..6ddac0704f 100644
--- a/usr/src/test/os-tests/tests/pf_key/kmc-update.sh
+++ b/usr/src/test/os-tests/tests/pf_key/kmc-update.sh
@@ -15,7 +15,7 @@
 # Copyright (c) 2018, Joyent, Inc.
 #
 
-if [[ `id -u` -ne 0 ]]; then
+if [[ `id -u` != 0 ]]; then
 	echo "Need to be root or have effective UID of root."
 	exit 255
 fi
diff --git a/usr/src/uts/common/inet/ip/sadb.c b/usr/src/uts/common/inet/ip/sadb.c
index 9ba7773656..0ec9e2bcee 100644
--- a/usr/src/uts/common/inet/ip/sadb.c
+++ b/usr/src/uts/common/inet/ip/sadb.c
@@ -23,6 +23,7 @@
  * Use is subject to license terms.
  * Copyright (c) 2012 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2018 Joyent, Inc.
+ * Copyright 2022 MNX Cloud, Inc.
  */
 
 #include <sys/types.h>
@@ -327,13 +328,16 @@ sadb_unlinkassoc(ipsa_t *ipsa)
 	ASSERT(ipsa->ipsa_linklock != NULL);
 	ASSERT(MUTEX_HELD(ipsa->ipsa_linklock));
 
+	/* Sometimes someone beats us here with the same SA. Check now. */
+	if (ipsa->ipsa_ptpn == NULL)
+		return;
+
 	/* These fields are protected by the link lock. */
 	*(ipsa->ipsa_ptpn) = ipsa->ipsa_next;
 	if (ipsa->ipsa_next != NULL) {
 		ipsa->ipsa_next->ipsa_ptpn = ipsa->ipsa_ptpn;
 		ipsa->ipsa_next = NULL;
 	}
-
 	ipsa->ipsa_ptpn = NULL;
 
 	/* This may destroy the SA. */