Module Name: src Committed By: martin Date: Fri Dec 1 09:21:15 UTC 2017
Modified Files: src/sys/netipsec [netbsd-8]: key.c Log Message: Pull up following revision(s) (requested by christos in ticket #415): sys/netipsec/key.c: revision 1.244 sys/netipsec/key.c: revision 1.245 Use KDASSERT for mutex_ownable Because mutex_ownable is not cheap. Fix a deadlock happening if !NET_MPSAFE If NET_MPSAFE isn't set, key_timehandler_work is executed with holding softnet_lock. This means that localcount_drain can be called with holding softnet_lock resulting in a deadlock that localcount_drain waits for packet processing to release a reference to SP/SA while network processing is prevented by softnet_lock. Fix the deadlock by not taking softnet_lock in key_timehandler_work. It's okay because IPsec is MP-safe even if !NET_MPSAFE. Note that the change also needs to enable pserialize_perform because the IPsec code can be run in parallel now. Reported by christos@ To generate a diff of this commit: cvs rdiff -u -r1.163.2.4 -r1.163.2.5 src/sys/netipsec/key.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/netipsec/key.c diff -u src/sys/netipsec/key.c:1.163.2.4 src/sys/netipsec/key.c:1.163.2.5 --- src/sys/netipsec/key.c:1.163.2.4 Thu Nov 30 15:57:37 2017 +++ src/sys/netipsec/key.c Fri Dec 1 09:21:15 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: key.c,v 1.163.2.4 2017/11/30 15:57:37 martin Exp $ */ +/* $NetBSD: key.c,v 1.163.2.5 2017/12/01 09:21:15 martin Exp $ */ /* $FreeBSD: src/sys/netipsec/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 $ */ @@ -32,7 +32,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.163.2.4 2017/11/30 15:57:37 martin Exp $"); +__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.163.2.5 2017/12/01 09:21:15 martin Exp $"); /* * This code is referred to RFC 2367 @@ -800,7 +800,6 @@ key_sp_refcnt(const struct secpolicy *sp return 0; } -#ifdef NET_MPSAFE static void key_spd_pserialize_perform(void) { @@ -818,7 +817,6 @@ key_spd_pserialize_perform(void) key_spd.psz_performing = false; cv_broadcast(&key_spd.cv_psz); } -#endif /* * Remove the sp from the key_spd.splist and wait for references to the sp @@ -836,10 +834,8 @@ key_unlink_sp(struct secpolicy *sp) /* Invalidate all cached SPD pointers in the PCBs. */ ipsec_invalpcbcacheall(); -#ifdef NET_MPSAFE - KASSERT(mutex_ownable(softnet_lock)); + KDASSERT(mutex_ownable(softnet_lock)); key_spd_pserialize_perform(); -#endif localcount_drain(&sp->localcount, &key_spd.cv_lc, &key_spd.lock); } @@ -1493,7 +1489,6 @@ key_freesp_so(struct secpolicy **sp) } #endif -#ifdef NET_MPSAFE static void key_sad_pserialize_perform(void) { @@ -1511,7 +1506,6 @@ key_sad_pserialize_perform(void) key_sad.psz_performing = false; cv_broadcast(&key_sad.cv_psz); } -#endif /* * Remove the sav from the savlist of its sah and wait for references to the sav @@ -1525,10 +1519,8 @@ key_unlink_sav(struct secasvar *sav) SAVLIST_WRITER_REMOVE(sav); -#ifdef NET_MPSAFE - KASSERT(mutex_ownable(softnet_lock)); + KDASSERT(mutex_ownable(softnet_lock)); key_sad_pserialize_perform(); -#endif localcount_drain(&sav->localcount, &key_sad.cv_lc, &key_sad.lock); } @@ -1567,10 +1559,8 @@ key_destroy_sav_with_ref(struct secasvar KEY_SA_UNREF(&sav); mutex_enter(&key_sad.lock); -#ifdef NET_MPSAFE - KASSERT(mutex_ownable(softnet_lock)); + KDASSERT(mutex_ownable(softnet_lock)); key_sad_pserialize_perform(); -#endif localcount_drain(&sav->localcount, &key_sad.cv_lc, &key_sad.lock); mutex_exit(&key_sad.lock); @@ -3048,10 +3038,8 @@ key_unlink_sah(struct secashead *sah) /* Remove from the sah list */ SAHLIST_WRITER_REMOVE(sah); -#ifdef NET_MPSAFE - KASSERT(mutex_ownable(softnet_lock)); + KDASSERT(mutex_ownable(softnet_lock)); key_sad_pserialize_perform(); -#endif localcount_drain(&sah->localcount, &key_sad.cv_lc, &key_sad.lock); } @@ -4862,13 +4850,10 @@ static void key_timehandler_work(struct work *wk, void *arg) { time_t now = time_uptime; - IPSEC_DECLARE_LOCK_VARIABLE; /* We can allow enqueuing another work at this point */ atomic_swap_uint(&key_timehandler_work_enqueued, 0); - IPSEC_ACQUIRE_GLOBAL_LOCKS(); - key_timehandler_spd(now); key_timehandler_sad(now); key_timehandler_acq(now); @@ -4879,7 +4864,6 @@ key_timehandler_work(struct work *wk, vo /* do exchange to tick time !! */ callout_reset(&key_timehandler_ch, hz, key_timehandler, NULL); - IPSEC_RELEASE_GLOBAL_LOCKS(); return; }