On Tue, Apr 07, 2020 at 06:38:11PM +0300, Vitaliy Makkoveev wrote: > On Tue, Apr 07, 2020 at 04:43:55PM +0200, Martin Pieuchot wrote: > > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote: > > > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4) > > > code has some NET_LOCK() dances which make it unsafe. [...] > > > > The easiest way to fix that is to move if_detach() out of pppx_if_destroy(). > > Yes, but we are going to move pppx(4) code out from locking. My diff > prevents concurent access to pxi_if by pppx_del_session(). Yes > pppx_del_session() can grab pxi_if owned by other threads (in the > future, after KERNEL_LOCK() will gone), but after refcounters this will > be fine and we can push NET_LOCK() deeper. >
In other words my diff kills the unlocked gap between pppx_if_find() and pppx_if_destroy(). Concurent pppx_del_session() can grab pppx_if (not for now) in this gap. Also I wish to avoid dances with twice bumped refcounter here in future. > > It generally makes sense to call if_detach() first then free/close the > > descriptor of a driver. However some drivers have callbacks and in that > > case you might want to teardown those first then call if_detach(). > > > > All the code from pppx_if_destroy() and pppx_add_session() has the wrong > ordered netif usage. > > > if_detach() will require the NET_LOCK() for some time. However > > pseudo-driver should start protecting their own data structure with > > different locks. > >
