From 7923b91c566a861898e3bcf7fcee8902a1507c4a Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Mon, 22 Jun 2020 16:02:17 +0200 Subject: [PATCH] pppoe: disc: document who holds references to the net object While working on another patch, I had a bug that *seemed* to be related to a use-after-free on the net structure, leading to a faulty patch removing the disc_stop refcount decrement, as I missed that the init_net function initialized the refcount to 1 and not 0. The bug was in my code. This patch adds simple comments the reference handling to make it a bit more obvious what is going on. Signed-off-by: Simon Chopin --- accel-pppd/ctrl/pppoe/disc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/accel-pppd/ctrl/pppoe/disc.c b/accel-pppd/ctrl/pppoe/disc.c index a0a21751..ff5f7fe7 100644 --- a/accel-pppd/ctrl/pppoe/disc.c +++ b/accel-pppd/ctrl/pppoe/disc.c @@ -90,6 +90,7 @@ static struct disc_net *init_net(const struct ap_net *net) n->hnd.fd = sock; n->hnd.read = disc_read; n->net = net; + /* Initial self-reference, to be released at net shutdown */ n->refs = 1; triton_context_register(&n->ctx, NULL); @@ -182,6 +183,7 @@ int pppoe_disc_start(struct pppoe_serv_t *serv) rb_link_node(&serv->node, parent, p); rb_insert_color(&serv->node, &t->root); + /* Register a reference to the net object */ __sync_add_and_fetch(&net->refs, 1); pthread_mutex_unlock(&t->lock); @@ -198,6 +200,7 @@ void pppoe_disc_stop(struct pppoe_serv_t *serv) rb_erase(&serv->node, &t->root); pthread_mutex_unlock(&t->lock); + /* Release the serv reference to the net object */ if (__sync_sub_and_fetch(&n->refs, 1) == 0) free_net(n); } @@ -280,6 +283,7 @@ static void disc_stop(struct disc_net *net) pthread_mutex_unlock(&t->lock); } + /* Release the initial reference. */ if (__sync_sub_and_fetch(&net->refs, 1) == 0) free_net(net); }