search

xebd / accel-ppp

High performance PPTP/L2TP/PPPoE/IPoE server for Linux
GNU General Public License v2.0
296 stars 108 forks source link

pppoe: disc: document who holds references to the net object #141

Open laarmen opened 4 years ago

laarmen commented 4 years ago

REVISED PATCH

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.

INITIAL VERSION

pppoe: disc: do not free the net struct in disc_stop

Assuming there is only one server associated with the current net object, freeing the net object at the end of disc_stop could potentially create a use-after-free error since before that, we queued an asynchronous call to _serv_stop, which in turn calls pppoe_disc_stop.

Since pppoe_disc_stop acquires a pointer to the net object at the beginning of its run, and uses it all throughout as part of its locking, having free_net running alongside could result in the memory backing the lock being freed while the code is running.

laarmen commented 3 years ago

OK, this patch is actually wrong. I'll replace it with a patch that only adds comments and edit the title and description. Feel free to directly close the PR if you think them superfluous.