Re: pipex(4) fix: check session existence before creation
ok yasuoka On Mon, 6 Apr 2020 19:54:20 +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. > > 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 - 1.79 > +++ sys/net/if_pppx.c 6 Apr 2020 13:47:26 - > @@ -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 - 1.112 > +++ sys/net/pipex.c 6 Apr 2020 13:47:33 - > @@ -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(_session_pool, PR_WAITOK | PR_ZERO); > session->state = PIPEX_STATE_OPENED; >
Re: pipex(4) fix: check session existence before creation
On Mon, Apr 06, 2020 at 06:32:01PM +0300, Vitaliy Makkoveev wrote: > > > > On 6 Apr 2020, at 17:37, Claudio Jeker 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 - 1.79 > >> +++ sys/net/if_pppx.c 6 Apr 2020 13:47:26 - > >> @@ -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.c6 Apr 2020 13:14:04 - 1.112 > >> +++ sys/net/pipex.c6 Apr 2020 13:47:33 - > >> @@ -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(_session_pool, PR_WAITOK | PR_ZERO); > >>session->state = PIPEX_STATE_OPENED; > >> > > > > -- > > :wq Claudio > -- :wq Claudio
Re: pipex(4) fix: check session existence before creation
> On 6 Apr 2020, at 17:37, Claudio Jeker 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. > >> 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.c6 Apr 2020 12:31:30 - 1.79 >> +++ sys/net/if_pppx.c6 Apr 2020 13:47:26 - >> @@ -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 - 1.112 >> +++ sys/net/pipex.c 6 Apr 2020 13:47:33 - >> @@ -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(_session_pool, PR_WAITOK | PR_ZERO); >> session->state = PIPEX_STATE_OPENED; >> > > -- > :wq Claudio
Re: pipex(4) fix: check session existence before creation
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@ > 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 - 1.79 > +++ sys/net/if_pppx.c 6 Apr 2020 13:47:26 - > @@ -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 - 1.112 > +++ sys/net/pipex.c 6 Apr 2020 13:47:33 - > @@ -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(_session_pool, PR_WAITOK | PR_ZERO); > session->state = PIPEX_STATE_OPENED; > -- :wq Claudio
pipex(4) fix: check session existence before creation
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. 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 - 1.79 +++ sys/net/if_pppx.c 6 Apr 2020 13:47:26 - @@ -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 - 1.112 +++ sys/net/pipex.c 6 Apr 2020 13:47:33 - @@ -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(_session_pool, PR_WAITOK | PR_ZERO); session->state = PIPEX_STATE_OPENED;
