From 930af642678ee3facd16f9ced3f72a773a8889bd Mon Sep 17 00:00:00 2001 From: Dan McDonald Date: Thu, 7 Jan 2010 15:08:58 -0500 Subject: [PATCH] 6801336 race causes IPsec SAs to occasionally have dangling packets. --- usr/src/uts/common/inet/ip/ip_sadb.c | 26 ++++++++++++------- usr/src/uts/common/inet/ip/ipsecah.c | 28 ++++++++++---------- usr/src/uts/common/inet/ip/ipsecesp.c | 28 ++++++++++---------- usr/src/uts/common/inet/ip/sadb.c | 48 ++++++++++++++++++++++------------- usr/src/uts/common/inet/sadb.h | 4 +-- 5 files changed, 80 insertions(+), 54 deletions(-) diff --git a/usr/src/uts/common/inet/ip/ip_sadb.c b/usr/src/uts/common/inet/ip/ip_sadb.c index e099d04427..c050dae45c 100644 --- a/usr/src/uts/common/inet/ip/ip_sadb.c +++ b/usr/src/uts/common/inet/ip/ip_sadb.c @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2009 Sun Microsystems, Inc. All rights reserved. + * Copyright 2010 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -744,11 +744,15 @@ ipsec_inbound_ah_sa(mblk_t *mp, ip_recv_attr_t *ira, ah_t **ahp) return (NULL); } - if (assoc->ipsa_state == IPSA_STATE_LARVAL && - sadb_set_lpkt(assoc, mp, ira)) { + if (assoc->ipsa_state == IPSA_STATE_LARVAL) { /* Not fully baked; swap the packet under a rock until then */ - IPSA_REFRELE(assoc); - return (NULL); + + mp = sadb_set_lpkt(assoc, mp, ira); + if (mp == NULL) { + IPSA_REFRELE(assoc); + return (NULL); + } + /* Looks like the SA is no longer LARVAL. */ } /* Are the IPsec fields initialized at all? */ @@ -883,11 +887,15 @@ ipsec_inbound_esp_sa(mblk_t *data_mp, ip_recv_attr_t *ira, esph_t **esphp) return (NULL); } - if (ipsa->ipsa_state == IPSA_STATE_LARVAL && - sadb_set_lpkt(ipsa, data_mp, ira)) { + if (ipsa->ipsa_state == IPSA_STATE_LARVAL) { /* Not fully baked; swap the packet under a rock until then */ - IPSA_REFRELE(ipsa); - return (NULL); + + data_mp = sadb_set_lpkt(ipsa, data_mp, ira); + if (data_mp == NULL) { + IPSA_REFRELE(ipsa); + return (NULL); + } + /* Looks like the SA is no longer LARVAL. */ } /* Are the IPsec fields initialized at all? */ diff --git a/usr/src/uts/common/inet/ip/ipsecah.c b/usr/src/uts/common/inet/ip/ipsecah.c index a511b85ff4..d303b38a25 100644 --- a/usr/src/uts/common/inet/ip/ipsecah.c +++ b/usr/src/uts/common/inet/ip/ipsecah.c @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2009 Sun Microsystems, Inc. All rights reserved. + * Copyright 2010 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -926,25 +926,27 @@ ah_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi, } /* Else sadb_common_add unlinks it for me! */ } - lpkt = NULL; - if (larval != NULL) - lpkt = sadb_clear_lpkt(larval); + if (larval != NULL) { + /* + * Hold again, because sadb_common_add() consumes a reference, + * and we don't want to clear_lpkt() without a reference. + */ + IPSA_REFHOLD(larval); + } rc = sadb_common_add(ahstack->ah_pfkey_q, mp, samsg, ksi, primary, secondary, larval, clone, is_inbound, diagnostic, ns, &ahstack->ah_sadb); - if (lpkt != NULL) { + if (larval != NULL) { if (rc == 0) { - rc = !taskq_dispatch(ah_taskq, inbound_task, lpkt, - TQ_NOSLEEP); - } - if (rc != 0) { - lpkt = ip_recv_attr_free_mblk(lpkt); - ip_drop_packet(lpkt, B_TRUE, NULL, - DROPPER(ipss, ipds_sadb_inlarval_timeout), - &ahstack->ah_dropper); + lpkt = sadb_clear_lpkt(larval); + if (lpkt != NULL) { + rc = !taskq_dispatch(ah_taskq, inbound_task, + lpkt, TQ_NOSLEEP); + } } + IPSA_REFRELE(larval); } /* diff --git a/usr/src/uts/common/inet/ip/ipsecesp.c b/usr/src/uts/common/inet/ip/ipsecesp.c index abf3e8816c..539eaef17c 100644 --- a/usr/src/uts/common/inet/ip/ipsecesp.c +++ b/usr/src/uts/common/inet/ip/ipsecesp.c @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2009 Sun Microsystems, Inc. All rights reserved. + * Copyright 2010 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -3450,25 +3450,27 @@ esp_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi, } /* Else sadb_common_add unlinks it for me! */ } - lpkt = NULL; - if (larval != NULL) - lpkt = sadb_clear_lpkt(larval); + if (larval != NULL) { + /* + * Hold again, because sadb_common_add() consumes a reference, + * and we don't want to clear_lpkt() without a reference. + */ + IPSA_REFHOLD(larval); + } rc = sadb_common_add(espstack->esp_pfkey_q, mp, samsg, ksi, primary, secondary, larval, clone, is_inbound, diagnostic, espstack->ipsecesp_netstack, &espstack->esp_sadb); - if (lpkt != NULL) { + if (larval != NULL) { if (rc == 0) { - rc = !taskq_dispatch(esp_taskq, inbound_task, - lpkt, TQ_NOSLEEP); - } - if (rc != 0) { - lpkt = ip_recv_attr_free_mblk(lpkt); - ip_drop_packet(lpkt, B_TRUE, NULL, - DROPPER(ipss, ipds_sadb_inlarval_timeout), - &espstack->esp_dropper); + lpkt = sadb_clear_lpkt(larval); + if (lpkt != NULL) { + rc = !taskq_dispatch(esp_taskq, inbound_task, + lpkt, TQ_NOSLEEP); + } } + IPSA_REFRELE(larval); } /* diff --git a/usr/src/uts/common/inet/ip/sadb.c b/usr/src/uts/common/inet/ip/sadb.c index 3d4ec0821e..6c46f53ac4 100644 --- a/usr/src/uts/common/inet/ip/sadb.c +++ b/usr/src/uts/common/inet/ip/sadb.c @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2009 Sun Microsystems, Inc. All rights reserved. + * Copyright 2010 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -6886,21 +6886,25 @@ bail: */ /* - * sadb_set_lpkt: Return TRUE if we can swap in a value to ipsa->ipsa_lpkt and - * freemsg the previous value. Return FALSE if we lost the race and the SA is - * in a non-LARVAL state. We also return FALSE if we can't allocate the attrmp. + * sadb_set_lpkt: + * + * Returns the passed-in packet if the SA is no longer larval. + * + * Returns NULL if the SA is larval, and needs to be swapped into the SA for + * processing after an SADB_UPDATE. */ -boolean_t +mblk_t * sadb_set_lpkt(ipsa_t *ipsa, mblk_t *npkt, ip_recv_attr_t *ira) { mblk_t *opkt; - netstack_t *ns = ira->ira_ill->ill_ipst->ips_netstack; - ipsec_stack_t *ipss = ns->netstack_ipsec; - boolean_t is_larval; mutex_enter(&ipsa->ipsa_lock); - is_larval = (ipsa->ipsa_state == IPSA_STATE_LARVAL); - if (is_larval) { + opkt = ipsa->ipsa_lpkt; + if (ipsa->ipsa_state == IPSA_STATE_LARVAL) { + /* + * Consume npkt and place it in the LARVAL SA's inbound + * packet slot. + */ mblk_t *attrmp; attrmp = ip_recv_attr_to_mblk(ira); @@ -6911,27 +6915,37 @@ sadb_set_lpkt(ipsa_t *ipsa, mblk_t *npkt, ip_recv_attr_t *ira) ip_drop_input("ipIfStatsInDiscards", npkt, ill); freemsg(npkt); opkt = NULL; - is_larval = B_FALSE; } else { ASSERT(attrmp->b_cont == NULL); attrmp->b_cont = npkt; - npkt = attrmp; - opkt = ipsa->ipsa_lpkt; - ipsa->ipsa_lpkt = npkt; + ipsa->ipsa_lpkt = attrmp; } + npkt = NULL; } else { - /* We lost the race. */ - opkt = NULL; + /* + * If not larval, we lost the race. NOTE: ipsa_lpkt may still + * have been non-NULL in the non-larval case, because of + * inbound packets arriving prior to sadb_common_add() + * transferring the SA completely out of larval state, but + * after lpkt was grabbed by the AH/ESP-specific add routines. + * We should clear the old ipsa_lpkt in this case to make sure + * that it doesn't linger on the now-MATURE IPsec SA, or get + * picked up as an out-of-order packet. + */ + ipsa->ipsa_lpkt = NULL; } mutex_exit(&ipsa->ipsa_lock); if (opkt != NULL) { + ipsec_stack_t *ipss; + + ipss = ira->ira_ill->ill_ipst->ips_netstack->netstack_ipsec; opkt = ip_recv_attr_free_mblk(opkt); ip_drop_packet(opkt, B_TRUE, ira->ira_ill, DROPPER(ipss, ipds_sadb_inlarval_replace), &ipss->ipsec_sadb_dropper); } - return (is_larval); + return (npkt); } /* diff --git a/usr/src/uts/common/inet/sadb.h b/usr/src/uts/common/inet/sadb.h index 7a45a41b85..73ffb1cf72 100644 --- a/usr/src/uts/common/inet/sadb.h +++ b/usr/src/uts/common/inet/sadb.h @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2009 Sun Microsystems, Inc. All rights reserved. + * Copyright 2010 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -784,7 +784,7 @@ void sadb_ager(sadb_t *, queue_t *, int, netstack_t *); timeout_id_t sadb_retimeout(hrtime_t, queue_t *, void (*)(void *), void *, uint_t *, uint_t, short); void sadb_sa_refrele(void *target); -boolean_t sadb_set_lpkt(ipsa_t *, mblk_t *, ip_recv_attr_t *); +mblk_t *sadb_set_lpkt(ipsa_t *, mblk_t *, ip_recv_attr_t *); mblk_t *sadb_clear_lpkt(ipsa_t *); void sadb_buf_pkt(ipsa_t *, mblk_t *, ip_recv_attr_t *); void sadb_clear_buf_pkt(void *ipkt); -- 2.11.4.GIT