d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve the locking of address lists by adding some asserts and

Browse Source 
rearranging the addition of address such that the lock is not
given up during checking and adding.

MFC after:		1 week
This commit is contained in:
Michael Tuexen 2020-07-17 15:09:49 +00:00
parent f3856e6881
commit 7f0ad2274b
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=363275
4 changed files with 55 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
sys/netinet/sctp_lock_bsd.h
View File

@ -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", \

64
sys/netinet/sctp_pcb.c
View File

@ -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_addr_lock)
/* 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, uint32_t ifn_index,
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, uint32_t ifn_index,
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, uint32_t ifn_index,
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, uint32_t ifn_index,
* 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, uint32_t ifn_index,
}
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, uint32_t ifn_index,
}
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, uint32_t ifn_index,
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, uint32_t ifn_index,
(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, uint32_t ifn_index,
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 @@ sctp_pcb_finish(void)
* 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 @@ sctp_pcb_finish(void)
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));

8
sys/netinet/sctp_usrreq.c
View File

@ -1004,9 +1004,6 @@ sctp_fill_user_address(struct sockaddr *dst, struct sockaddr *src)
/*
* 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, uint32_t vrf_id)
* 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);

7
sys/netinet/sctputil.c
View File

@ -5296,8 +5296,11 @@ sctp_find_ifa_by_addr(struct sockaddr *addr, uint32_t vrf_id, int holds_lock)
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 vrf_id)
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);