On Mon, Apr 06, 2020 at 06:32:01PM +0300, Vitaliy Makkoveev wrote: > > > > On 6 Apr 2020, at 17:37, Claudio Jeker <[email protected]> wrote: > > > > On Mon, Apr 06, 2020 at 07:54:20PM +0300, Vitaliy Makkoveev wrote: > >> Deny to create pipex_session which is already exist. Newly created > >> session will be placed to list head so the caller of > >> pipex_*_lookup_session() will receive wrong session. > > > > I think in the pppx(4) case the code is already doing this check in the > > RBT_FIND() on line 835. Still I think this is a good thing to add. > > OK claudio@ > > > > In pppx(4) layer not in pipex(4). Without this check pppx(4) can > override pppac(4) owned session.
Yes, the pppac(4) version does not do the check. I'm not sure sure if it is valid to use both pppx and pppac at the same time. In the end doing the check feels right. > > > >> Index: sys/net/if_pppx.c > >> =================================================================== > >> RCS file: /cvs/src/sys/net/if_pppx.c,v > >> retrieving revision 1.79 > >> diff -u -p -r1.79 if_pppx.c > >> --- sys/net/if_pppx.c 6 Apr 2020 12:31:30 -0000 1.79 > >> +++ sys/net/if_pppx.c 6 Apr 2020 13:47:26 -0000 > >> @@ -719,6 +719,11 @@ pppx_add_session(struct pppx_dev *pxd, s > >> return (EPROTONOSUPPORT); > >> } > >> > >> + session = pipex_lookup_by_session_id(req->pr_protocol, > >> + req->pr_session_id); > >> + if (session) > >> + return (EEXIST); > >> + > >> pxi = pool_get(pppx_if_pl, PR_WAITOK | PR_ZERO); > >> if (pxi == NULL) > >> return (ENOMEM); > >> Index: sys/net/pipex.c > >> =================================================================== > >> RCS file: /cvs/src/sys/net/pipex.c,v > >> retrieving revision 1.112 > >> diff -u -p -r1.112 pipex.c > >> --- sys/net/pipex.c 6 Apr 2020 13:14:04 -0000 1.112 > >> +++ sys/net/pipex.c 6 Apr 2020 13:47:33 -0000 > >> @@ -312,6 +312,11 @@ pipex_add_session(struct pipex_session_r > >> return (EPROTONOSUPPORT); > >> } > >> > >> + session = pipex_lookup_by_session_id(req->pr_protocol, > >> + req->pr_session_id); > >> + if (session) > >> + return (EEXIST); > >> + > >> /* prepare a new session */ > >> session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO); > >> session->state = PIPEX_STATE_OPENED; > >> > > > > -- > > :wq Claudio > -- :wq Claudio
