Author: tuexen
Date: Fri Jul 17 15:09:49 2020
New Revision: 363275
URL: https://svnweb.freebsd.org/changeset/base/363275
Log:
Improve the locking of address lists by adding some asserts and
rearranging the addition of address such that the lock is not
given up during checking and adding.
MFC after: 1 week
Modified:
head/sys/netinet/sctp_lock_bsd.h
head/sys/netinet/sctp_pcb.c
head/sys/netinet/sctp_usrreq.c
head/sys/netinet/sctputil.c
Modified: head/sys/netinet/sctp_lock_bsd.h
==============================================================================
--- head/sys/netinet/sctp_lock_bsd.h Fri Jul 17 14:51:51 2020
(r363274)
+++ head/sys/netinet/sctp_lock_bsd.h Fri Jul 17 15:09:49 2020
(r363275)
@@ -177,6 +177,13 @@ __FBSDID("$FreeBSD$");
rw_wunlock(&SCTP_BASE_INFO(ipi_addr_mtx)); \
} while (0)
+#define SCTP_IPI_ADDR_LOCK_ASSERT() do { \
+ rw_assert(&SCTP_BASE_INFO(ipi_addr_mtx), RA_LOCKED); \
+} while (0)
+
+#define SCTP_IPI_ADDR_WLOCK_ASSERT() do { \
+ rw_assert(&SCTP_BASE_INFO(ipi_addr_mtx), RA_WLOCKED); \
+} while (0)
#define SCTP_IPI_ITERATOR_WQ_INIT() do { \
mtx_init(&sctp_it_ctl.ipi_iterator_wq_mtx, "sctp-it-wq", \
Modified: head/sys/netinet/sctp_pcb.c
==============================================================================
--- head/sys/netinet/sctp_pcb.c Fri Jul 17 14:51:51 2020 (r363274)
+++ head/sys/netinet/sctp_pcb.c Fri Jul 17 15:09:49 2020 (r363275)
@@ -200,6 +200,7 @@ sctp_find_ifn(void *ifn, uint32_t ifn_index)
* We assume the lock is held for the addresses if that's wrong
* problems could occur :-)
*/
+ SCTP_IPI_ADDR_LOCK_ASSERT();
hash_ifn_head = &SCTP_BASE_INFO(vrf_ifn_hash)[(ifn_index &
SCTP_BASE_INFO(vrf_ifn_hashmark))];
LIST_FOREACH(sctp_ifnp, hash_ifn_head, next_bucket) {
if (sctp_ifnp->ifn_index == ifn_index) {
@@ -295,12 +296,16 @@ sctp_delete_ifn(struct sctp_ifn *sctp_ifnp, int hold_a
/* Not in the list.. sorry */
return;
}
- if (hold_addr_lock == 0)
+ if (hold_addr_lock == 0) {
SCTP_IPI_ADDR_WLOCK();
+ } else {
+ SCTP_IPI_ADDR_WLOCK_ASSERT();
+ }
LIST_REMOVE(sctp_ifnp, next_bucket);
LIST_REMOVE(sctp_ifnp, next_ifn);
- if (hold_addr_lock == 0)
+ if (hold_addr_lock == 0) {
SCTP_IPI_ADDR_WUNLOCK();
+ }
/* Take away the reference, and possibly free it */
sctp_free_ifn(sctp_ifnp);
}
@@ -486,8 +491,8 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
int dynamic_add)
{
struct sctp_vrf *vrf;
- struct sctp_ifn *sctp_ifnp = NULL;
- struct sctp_ifa *sctp_ifap = NULL;
+ struct sctp_ifn *sctp_ifnp, *new_sctp_ifnp;
+ struct sctp_ifa *sctp_ifap, *new_sctp_ifap;
struct sctp_ifalist *hash_addr_head;
struct sctp_ifnlist *hash_ifn_head;
uint32_t hash_of_addr;
@@ -497,6 +502,23 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
SCTPDBG(SCTP_DEBUG_PCB4, "vrf_id 0x%x: adding address: ", vrf_id);
SCTPDBG_ADDR(SCTP_DEBUG_PCB4, addr);
#endif
+ SCTP_MALLOC(new_sctp_ifnp, struct sctp_ifn *,
+ sizeof(struct sctp_ifn), SCTP_M_IFN);
+ if (new_sctp_ifnp == NULL) {
+#ifdef INVARIANTS
+ panic("No memory for IFN");
+#endif
+ return (NULL);
+ }
+ SCTP_MALLOC(new_sctp_ifap, struct sctp_ifa *, sizeof(struct sctp_ifa),
SCTP_M_IFA);
+ if (new_sctp_ifap == NULL) {
+#ifdef INVARIANTS
+ panic("No memory for IFA");
+#endif
+ SCTP_FREE(new_sctp_ifnp, SCTP_M_IFN);
+ return (NULL);
+ }
+
SCTP_IPI_ADDR_WLOCK();
sctp_ifnp = sctp_find_ifn(ifn, ifn_index);
if (sctp_ifnp) {
@@ -507,6 +529,8 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
vrf = sctp_allocate_vrf(vrf_id);
if (vrf == NULL) {
SCTP_IPI_ADDR_WUNLOCK();
+ SCTP_FREE(new_sctp_ifnp, SCTP_M_IFN);
+ SCTP_FREE(new_sctp_ifap, SCTP_M_IFA);
return (NULL);
}
}
@@ -516,15 +540,8 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
* build one and add it, can't hold lock until after malloc
* done though.
*/
- SCTP_IPI_ADDR_WUNLOCK();
- SCTP_MALLOC(sctp_ifnp, struct sctp_ifn *,
- sizeof(struct sctp_ifn), SCTP_M_IFN);
- if (sctp_ifnp == NULL) {
-#ifdef INVARIANTS
- panic("No memory for IFN");
-#endif
- return (NULL);
- }
+ sctp_ifnp = new_sctp_ifnp;
+ new_sctp_ifnp = NULL;
memset(sctp_ifnp, 0, sizeof(struct sctp_ifn));
sctp_ifnp->ifn_index = ifn_index;
sctp_ifnp->ifn_p = ifn;
@@ -540,7 +557,6 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
}
hash_ifn_head = &SCTP_BASE_INFO(vrf_ifn_hash)[(ifn_index &
SCTP_BASE_INFO(vrf_ifn_hashmark))];
LIST_INIT(&sctp_ifnp->ifalist);
- SCTP_IPI_ADDR_WLOCK();
LIST_INSERT_HEAD(hash_ifn_head, sctp_ifnp, next_bucket);
LIST_INSERT_HEAD(&vrf->ifnlist, sctp_ifnp, next_ifn);
atomic_add_int(&SCTP_BASE_INFO(ipi_count_ifns), 1);
@@ -567,6 +583,10 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
}
exit_stage_left:
SCTP_IPI_ADDR_WUNLOCK();
+ if (new_sctp_ifnp != NULL) {
+ SCTP_FREE(new_sctp_ifnp, SCTP_M_IFN);
+ }
+ SCTP_FREE(new_sctp_ifap, SCTP_M_IFA);
return (sctp_ifap);
} else {
if (sctp_ifap->ifn_p) {
@@ -593,14 +613,7 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
goto exit_stage_left;
}
}
- SCTP_IPI_ADDR_WUNLOCK();
- SCTP_MALLOC(sctp_ifap, struct sctp_ifa *, sizeof(struct sctp_ifa),
SCTP_M_IFA);
- if (sctp_ifap == NULL) {
-#ifdef INVARIANTS
- panic("No memory for IFA");
-#endif
- return (NULL);
- }
+ sctp_ifap = new_sctp_ifap;
memset(sctp_ifap, 0, sizeof(struct sctp_ifa));
sctp_ifap->ifn_p = sctp_ifnp;
atomic_add_int(&sctp_ifnp->refcount, 1);
@@ -660,7 +673,6 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
(sctp_ifap->src_is_loop == 0)) {
sctp_ifap->src_is_glob = 1;
}
- SCTP_IPI_ADDR_WLOCK();
hash_addr_head = &vrf->vrf_addr_hash[(hash_of_addr &
vrf->vrf_addr_hashmark)];
LIST_INSERT_HEAD(hash_addr_head, sctp_ifap, next_bucket);
sctp_ifap->refcount = 1;
@@ -672,6 +684,10 @@ sctp_add_addr_to_vrf(uint32_t vrf_id, void *ifn, uint3
sctp_ifnp->registered_af = new_ifn_af;
}
SCTP_IPI_ADDR_WUNLOCK();
+ if (new_sctp_ifnp != NULL) {
+ SCTP_FREE(new_sctp_ifnp, SCTP_M_IFN);
+ }
+
if (dynamic_add) {
/*
* Bump up the refcount so that when the timer completes it
@@ -5906,6 +5922,7 @@ retry:
* free the vrf/ifn/ifa lists and hashes (be sure address monitor is
* destroyed first).
*/
+ SCTP_IPI_ADDR_WLOCK();
vrf_bucket = &SCTP_BASE_INFO(sctp_vrfhash)[(SCTP_DEFAULT_VRFID &
SCTP_BASE_INFO(hashvrfmark))];
LIST_FOREACH_SAFE(vrf, vrf_bucket, next_vrf, nvrf) {
LIST_FOREACH_SAFE(ifn, &vrf->ifnlist, next_ifn, nifn) {
@@ -5925,6 +5942,7 @@ retry:
LIST_REMOVE(vrf, next_vrf);
SCTP_FREE(vrf, SCTP_M_VRF);
}
+ SCTP_IPI_ADDR_WUNLOCK();
/* free the vrf hashes */
SCTP_HASH_FREE(SCTP_BASE_INFO(sctp_vrfhash),
SCTP_BASE_INFO(hashvrfmark));
SCTP_HASH_FREE(SCTP_BASE_INFO(vrf_ifn_hash),
SCTP_BASE_INFO(vrf_ifn_hashmark));
Modified: head/sys/netinet/sctp_usrreq.c
==============================================================================
--- head/sys/netinet/sctp_usrreq.c Fri Jul 17 14:51:51 2020
(r363274)
+++ head/sys/netinet/sctp_usrreq.c Fri Jul 17 15:09:49 2020
(r363275)
@@ -1004,9 +1004,6 @@ sctp_fill_user_address(struct sockaddr *dst, struct so
-/*
- * NOTE: assumes addr lock is held
- */
static size_t
sctp_fill_up_addresses_vrf(struct sctp_inpcb *inp,
struct sctp_tcb *stcb,
@@ -1026,6 +1023,7 @@ sctp_fill_up_addresses_vrf(struct sctp_inpcb *inp,
#endif
struct sctp_vrf *vrf;
+ SCTP_IPI_ADDR_LOCK_ASSERT();
actual = 0;
if (limit == 0)
return (actual);
@@ -1258,9 +1256,6 @@ sctp_fill_up_addresses(struct sctp_inpcb *inp,
return (size);
}
-/*
- * NOTE: assumes addr lock is held
- */
static int
sctp_count_max_addresses_vrf(struct sctp_inpcb *inp, uint32_t vrf_id)
{
@@ -1274,6 +1269,7 @@ sctp_count_max_addresses_vrf(struct sctp_inpcb *inp, u
* bound-all case a TCB may NOT include the loopback or other
* addresses as well.
*/
+ SCTP_IPI_ADDR_LOCK_ASSERT();
vrf = sctp_find_vrf(vrf_id);
if (vrf == NULL) {
return (0);
Modified: head/sys/netinet/sctputil.c
==============================================================================
--- head/sys/netinet/sctputil.c Fri Jul 17 14:51:51 2020 (r363274)
+++ head/sys/netinet/sctputil.c Fri Jul 17 15:09:49 2020 (r363275)
@@ -5296,8 +5296,11 @@ sctp_find_ifa_by_addr(struct sockaddr *addr, uint32_t
struct sctp_ifalist *hash_head;
uint32_t hash_of_addr;
- if (holds_lock == 0)
+ if (holds_lock == 0) {
SCTP_IPI_ADDR_RLOCK();
+ } else {
+ SCTP_IPI_ADDR_LOCK_ASSERT();
+ }
vrf = sctp_find_vrf(vrf_id);
if (vrf == NULL) {
@@ -6429,7 +6432,7 @@ sctp_dynamic_set_primary(struct sockaddr *sa, uint32_t
struct sctp_ifa *ifa;
struct sctp_laddr *wi;
- ifa = sctp_find_ifa_by_addr(sa, vrf_id, 0);
+ ifa = sctp_find_ifa_by_addr(sa, vrf_id, SCTP_ADDR_NOT_LOCKED);
if (ifa == NULL) {
SCTP_LTRACE_ERR_RET(NULL, NULL, NULL, SCTP_FROM_SCTPUTIL,
EADDRNOTAVAIL);
return (EADDRNOTAVAIL);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
