On Fri, Aug 07, 2020 at 09:29:13PM +0100, Jason McIntyre wrote: > On Fri, Aug 07, 2020 at 10:19:05PM +0300, Vitaliy Makkoveev wrote: > > Some times ago we disabled in-kernel timeout for pppx(4) related > > pipex(4) sessions. We did this for prevent use after free issue caused > > by pipex_timer [1]. By default "idle-timeout" is not set in > > npppd.conf(5) and I guess this is reason for we forgot to describe this > > exception in npppd.conf(5). > > > > But looks like one user caught this [2]. So I propose to describe this > > in BUGS section of npppd.conf(5). > > > > Also current "idle-timeout" description looks incorrect. If this option > > is missing, there is not in-kernel timeout for this session, but > > npppd(8) uses it's own timeout for. And we can't configure this value. > > > > YASUOKA, what do you think? May be we can kill in-kernel timeout feature > > for pipex(4)?, and make npppd(8)'s idle timeout configurable by this > > option? > > > > 1. > > https://cvsweb.openbsd.org/src/sys/net/if_pppx.c?rev=1.78&content-type=text/x-cvsweb-markup > > 2. https://marc.info/?l=openbsd-misc&m=159655468504864&w=2 > > > > > > Index: usr.sbin/npppd/npppd/npppd.conf.5 > > =================================================================== > > RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v > > retrieving revision 1.27 > > diff -u -p -r1.27 npppd.conf.5 > > --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 -0000 > > 1.27 > > +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 19:17:00 -0000 > > @@ -699,3 +699,9 @@ The current version of > > .Xr npppd 8 > > does not support adding or removing tunnel settings or changing listener > > settings (listen address, port and l2tp-ipsec-require). > > +.Pp > > +This time > > +.Xr pppx 4 > > +does not allow to create sessions with non null > > +.Ic idle-timeout > > +option. > > >
Thanks for your feedback. My English is bad, so thanks for fixing. > is this an actual bug? i'm just asking - it might be that the > idle-timeout text is the best place to warn users, and not BUGS. It is pppx(4) related bug. Unfortunately it wasn't solved and we just disabled this feature to avoid panics. May be pipex(4) man page is the best place to describe this issue in BUGS section. > > regarding your text: > > - "this time" is better written as "At this time" or "currently". > - "allow to create" is not good sentence structure > > i think the text would read better something like: > > .Xr pppx 4 > does not allow sessions with > .Ic idle-timeout > set to any value other than 0. > I added this to pipex(4) BUGS section. > if the text was better placed in the idle-timeout section: > > This value must be 0 for > .Xr pppx 4 > sessions. And this to npppd.conf(5) idle-timeout section. Index: share/man/man4/pipex.4 =================================================================== RCS file: /cvs/src/share/man/man4/pipex.4,v retrieving revision 1.12 diff -u -p -r1.12 pipex.4 --- share/man/man4/pipex.4 3 Apr 2020 07:46:04 -0000 1.12 +++ share/man/man4/pipex.4 7 Aug 2020 20:54:32 -0000 @@ -288,3 +288,8 @@ The .Nm was written by .An Internet Initiative Japan Inc . +.Sh BUGS +.Xr pppx 4 +does not allow sessions with +.Ic pr_timeout_sec +set to any value other than 0. Index: usr.sbin/npppd/npppd/npppd.conf.5 =================================================================== RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v retrieving revision 1.27 diff -u -p -r1.27 npppd.conf.5 --- usr.sbin/npppd/npppd/npppd.conf.5 23 Apr 2020 21:10:54 -0000 1.27 +++ usr.sbin/npppd/npppd/npppd.conf.5 7 Aug 2020 20:54:32 -0000 @@ -325,6 +325,9 @@ The link is disconnected if there are no for more than the amount of the .Ar idle-timeout . The default is 0, which disables the idle timer. +This value must be 0 for +.Xr pppx 4 +sessions. .It Ic tcp-mss-adjust Ar yes | no If .Dq yes
