Fix two races which happen when netgraph is restructuring:
- Introduce ng_topo_mtx, a mutex to protect topology changes. - In ng_destroy_node() protect with ng_topo_mtx the process of checking and pointing at ng_deadnode. [1] - In ng_con_part2() check that our peer is not a ng_deadnode, and protect the check with ng_topo_mtx. - Add KASSERTs to ng_acquire_read/write, to make more understandible synopsis in case if called on ng_deadnode. Reported by: Roselyn Lee [1]
This commit is contained in:
parent
4be5933577
commit
ac5dd14182
@ -71,6 +71,9 @@ MODULE_VERSION(netgraph, NG_ABI_VERSION);
|
|||||||
static LIST_HEAD(, ng_node) ng_nodelist;
|
static LIST_HEAD(, ng_node) ng_nodelist;
|
||||||
static struct mtx ng_nodelist_mtx;
|
static struct mtx ng_nodelist_mtx;
|
||||||
|
|
||||||
|
/* Mutex to protect topology events. */
|
||||||
|
static struct mtx ng_topo_mtx;
|
||||||
|
|
||||||
#ifdef NETGRAPH_DEBUG
|
#ifdef NETGRAPH_DEBUG
|
||||||
static struct mtx ngq_mtx; /* protects the queue item list */
|
static struct mtx ngq_mtx; /* protects the queue item list */
|
||||||
|
|
||||||
@ -1044,14 +1047,25 @@ ng_findhook(node_p node, const char *name)
|
|||||||
void
|
void
|
||||||
ng_destroy_hook(hook_p hook)
|
ng_destroy_hook(hook_p hook)
|
||||||
{
|
{
|
||||||
hook_p peer = NG_HOOK_PEER(hook);
|
hook_p peer;
|
||||||
node_p node = NG_HOOK_NODE(hook);
|
node_p node;
|
||||||
|
|
||||||
if (hook == &ng_deadhook) { /* better safe than sorry */
|
if (hook == &ng_deadhook) { /* better safe than sorry */
|
||||||
printf("ng_destroy_hook called on deadhook\n");
|
printf("ng_destroy_hook called on deadhook\n");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
hook->hk_flags |= HK_INVALID; /* as soon as possible */
|
|
||||||
|
/*
|
||||||
|
* Protect divorce process with mutex, to avoid races on
|
||||||
|
* simultaneous disconnect.
|
||||||
|
*/
|
||||||
|
mtx_lock(&ng_topo_mtx);
|
||||||
|
|
||||||
|
hook->hk_flags |= HK_INVALID;
|
||||||
|
|
||||||
|
peer = NG_HOOK_PEER(hook);
|
||||||
|
node = NG_HOOK_NODE(hook);
|
||||||
|
|
||||||
if (peer && (peer != &ng_deadhook)) {
|
if (peer && (peer != &ng_deadhook)) {
|
||||||
/*
|
/*
|
||||||
* Set the peer to point to ng_deadhook
|
* Set the peer to point to ng_deadhook
|
||||||
@ -1065,13 +1079,17 @@ ng_destroy_hook(hook_p hook)
|
|||||||
* If it's already divorced from a node,
|
* If it's already divorced from a node,
|
||||||
* just free it.
|
* just free it.
|
||||||
*/
|
*/
|
||||||
/* nothing */
|
mtx_unlock(&ng_topo_mtx);
|
||||||
} else {
|
} else {
|
||||||
|
mtx_unlock(&ng_topo_mtx);
|
||||||
ng_rmhook_self(peer); /* Send it a surprise */
|
ng_rmhook_self(peer); /* Send it a surprise */
|
||||||
}
|
}
|
||||||
NG_HOOK_UNREF(peer); /* account for peer link */
|
NG_HOOK_UNREF(peer); /* account for peer link */
|
||||||
NG_HOOK_UNREF(hook); /* account for peer link */
|
NG_HOOK_UNREF(hook); /* account for peer link */
|
||||||
}
|
} else
|
||||||
|
mtx_unlock(&ng_topo_mtx);
|
||||||
|
|
||||||
|
mtx_assert(&ng_topo_mtx, MA_NOTOWNED);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Remove the hook from the node's list to avoid possible recursion
|
* Remove the hook from the node's list to avoid possible recursion
|
||||||
@ -1244,6 +1262,7 @@ ng_con_part3(node_p node, hook_p hook, void *arg1, int arg2)
|
|||||||
static void
|
static void
|
||||||
ng_con_part2(node_p node, hook_p hook, void *arg1, int arg2)
|
ng_con_part2(node_p node, hook_p hook, void *arg1, int arg2)
|
||||||
{
|
{
|
||||||
|
hook_p peer;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* When we run, we know that the node 'node' is locked for us.
|
* When we run, we know that the node 'node' is locked for us.
|
||||||
@ -1301,9 +1320,22 @@ ng_con_part2(node_p node, hook_p hook, void *arg1, int arg2)
|
|||||||
return ;
|
return ;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (ng_send_fn(hook->hk_peer->hk_node, hook->hk_peer,
|
|
||||||
&ng_con_part3, arg1, arg2)) {
|
/*
|
||||||
printf("failed in ng_con_part2(B)");
|
* Acquire topo mutex to avoid race with ng_destroy_hook().
|
||||||
|
*/
|
||||||
|
mtx_lock(&ng_topo_mtx);
|
||||||
|
peer = hook->hk_peer;
|
||||||
|
if (peer == &ng_deadhook) {
|
||||||
|
mtx_unlock(&ng_topo_mtx);
|
||||||
|
printf("failed in ng_con_part2(B)\n");
|
||||||
|
ng_destroy_hook(hook);
|
||||||
|
return ;
|
||||||
|
}
|
||||||
|
mtx_unlock(&ng_topo_mtx);
|
||||||
|
|
||||||
|
if (ng_send_fn(peer->hk_node, peer, &ng_con_part3, arg1, arg2)) {
|
||||||
|
printf("failed in ng_con_part2(C)\n");
|
||||||
ng_destroy_hook(hook); /* also zaps peer */
|
ng_destroy_hook(hook); /* also zaps peer */
|
||||||
return ;
|
return ;
|
||||||
}
|
}
|
||||||
@ -1976,6 +2008,8 @@ ng_queue_rw(struct ng_queue * ngq, item_p item, int rw)
|
|||||||
static __inline item_p
|
static __inline item_p
|
||||||
ng_acquire_read(struct ng_queue *ngq, item_p item)
|
ng_acquire_read(struct ng_queue *ngq, item_p item)
|
||||||
{
|
{
|
||||||
|
KASSERT(ngq != &ng_deadnode.nd_input_queue,
|
||||||
|
("%s: working on deadnode", __func__));
|
||||||
|
|
||||||
/* ######### Hack alert ######### */
|
/* ######### Hack alert ######### */
|
||||||
atomic_add_long(&ngq->q_flags, READER_INCREMENT);
|
atomic_add_long(&ngq->q_flags, READER_INCREMENT);
|
||||||
@ -2014,6 +2048,9 @@ ng_acquire_read(struct ng_queue *ngq, item_p item)
|
|||||||
static __inline item_p
|
static __inline item_p
|
||||||
ng_acquire_write(struct ng_queue *ngq, item_p item)
|
ng_acquire_write(struct ng_queue *ngq, item_p item)
|
||||||
{
|
{
|
||||||
|
KASSERT(ngq != &ng_deadnode.nd_input_queue,
|
||||||
|
("%s: working on deadnode", __func__));
|
||||||
|
|
||||||
restart:
|
restart:
|
||||||
mtx_lock_spin(&(ngq->q_mtx));
|
mtx_lock_spin(&(ngq->q_mtx));
|
||||||
/*
|
/*
|
||||||
@ -3013,6 +3050,8 @@ ngb_mod_event(module_t mod, int event, void *data)
|
|||||||
MTX_DEF);
|
MTX_DEF);
|
||||||
mtx_init(&ng_idhash_mtx, "netgraph idhash mutex", NULL,
|
mtx_init(&ng_idhash_mtx, "netgraph idhash mutex", NULL,
|
||||||
MTX_DEF);
|
MTX_DEF);
|
||||||
|
mtx_init(&ng_topo_mtx, "netgraph topology mutex", NULL,
|
||||||
|
MTX_DEF);
|
||||||
#ifdef NETGRAPH_DEBUG
|
#ifdef NETGRAPH_DEBUG
|
||||||
mtx_init(&ngq_mtx, "netgraph item list mutex", NULL,
|
mtx_init(&ngq_mtx, "netgraph item list mutex", NULL,
|
||||||
MTX_DEF);
|
MTX_DEF);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user