pipex_iface_fini() release multicast session under NET_LOCK()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

pipex_iface_fini() release multicast session under NET_LOCK()

Vitaliy Makkoveev-2
We are going to lock the whole pipex(4) by NET_LOCK(). So move
`multicast_session' freeing undet NET_LOCK() too.

Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.120
diff -u -p -r1.120 pipex.c
--- sys/net/pipex.c 17 Jul 2020 08:57:27 -0000 1.120
+++ sys/net/pipex.c 17 Jul 2020 13:23:16 -0000
@@ -192,8 +192,8 @@ pipex_iface_stop(struct pipex_iface_cont
 void
 pipex_iface_fini(struct pipex_iface_context *pipex_iface)
 {
- pool_put(&pipex_session_pool, pipex_iface->multicast_session);
  NET_LOCK();
+ pool_put(&pipex_session_pool, pipex_iface->multicast_session);
  pipex_iface_stop(pipex_iface);
  NET_UNLOCK();
 }

Reply | Threaded
Open this post in threaded view
|

Re: pipex_iface_fini() release multicast session under NET_LOCK()

Martin Pieuchot
On 17/07/20(Fri) 16:29, Vitaliy Makkoveev wrote:
> We are going to lock the whole pipex(4) by NET_LOCK(). So move
> `multicast_session' freeing undet NET_LOCK() too.

pipex_iface_fini() should be called on the last reference of the
descriptor.  So this shouldn't be necessary.  If there's an issue
with the current order of the operations, we should certainly fix
it differently.

> Index: sys/net/pipex.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 pipex.c
> --- sys/net/pipex.c 17 Jul 2020 08:57:27 -0000 1.120
> +++ sys/net/pipex.c 17 Jul 2020 13:23:16 -0000
> @@ -192,8 +192,8 @@ pipex_iface_stop(struct pipex_iface_cont
>  void
>  pipex_iface_fini(struct pipex_iface_context *pipex_iface)
>  {
> - pool_put(&pipex_session_pool, pipex_iface->multicast_session);
>   NET_LOCK();
> + pool_put(&pipex_session_pool, pipex_iface->multicast_session);
>   pipex_iface_stop(pipex_iface);
>   NET_UNLOCK();
>  }
>

Reply | Threaded
Open this post in threaded view
|

Re: pipex_iface_fini() release multicast session under NET_LOCK()

Vitaliy Makkoveev-2
On Tue, Jul 28, 2020 at 10:23:08AM +0200, Martin Pieuchot wrote:
> On 17/07/20(Fri) 16:29, Vitaliy Makkoveev wrote:
> > We are going to lock the whole pipex(4) by NET_LOCK(). So move
> > `multicast_session' freeing undet NET_LOCK() too.
>
> pipex_iface_fini() should be called on the last reference of the
> descriptor.  So this shouldn't be necessary.  If there's an issue
> with the current order of the operations, we should certainly fix
> it differently.
>

`multicast_session' can be processed by pipexintr() while we do
ppppacclose(). There is no wrong order, npppd(8) or another userland
prgramm can be killed in any time.

> > Index: sys/net/pipex.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pipex.c,v
> > retrieving revision 1.120
> > diff -u -p -r1.120 pipex.c
> > --- sys/net/pipex.c 17 Jul 2020 08:57:27 -0000 1.120
> > +++ sys/net/pipex.c 17 Jul 2020 13:23:16 -0000
> > @@ -192,8 +192,8 @@ pipex_iface_stop(struct pipex_iface_cont
> >  void
> >  pipex_iface_fini(struct pipex_iface_context *pipex_iface)
> >  {
> > - pool_put(&pipex_session_pool, pipex_iface->multicast_session);
> >   NET_LOCK();
> > + pool_put(&pipex_session_pool, pipex_iface->multicast_session);
> >   pipex_iface_stop(pipex_iface);
> >   NET_UNLOCK();
> >  }
> >
>