On Sat, Dec 09, 2017 at 08:26:58PM +0100, Sebastian Benoit wrote:
>
> Hi,
>
> sorry for the delay, and thanks for the report and the analysis!
>
> here is a slighty changed version:
>
> diff --git usr.sbin/relayd/hce.c usr.sbin/relayd/hce.c
> index 5c5ee6f3013..bb2903c8dfa 100644
> --- usr.sbin/relayd/hce.c
> +++ usr.sbin/relayd/hce.c
> @@ -80,11 +80,11 @@ hce_setup_events(void)
> struct timeval tv;
> struct table *table;
>
> - if (!(TAILQ_EMPTY(env->sc_tables) ||
> - event_initialized(&env->sc_ev))) {
> + if (!event_initialized(&env->sc_ev)) {
> evtimer_set(&env->sc_ev, hce_launch_checks, env);
> bzero(&tv, sizeof(tv));
> - evtimer_add(&env->sc_ev, &tv);
> + if (!TAILQ_EMPTY(env->sc_tables))
> + evtimer_add(&env->sc_ev, &tv);
> }
>
> if (env->sc_conf.flags & F_TLS) {
>
>
> ok?
I think then you want to have a similar TAILQ_EMPTY check in
hce_launch_checks(). Because doing an IMSG_CTL_POLL will trigger the
evtimer_add() in there. Either we always run the timer or we make sure we
never run the timer for an empty queue.
>
> Hiltjo Posthuma([email protected]) on 2017.11.18 17:06:33 +0100:
> > On Sun, Oct 15, 2017 at 02:05:09PM +0200, Hiltjo Posthuma wrote:
> > > Hey,
> > >
> > > I can reproduce it, below is some backtrace with debug symbols and source
> > > and hopefully a few useful notes. Sorry: no patch attached ;)
> > >
> > >
> > > (gdb) bt
> > > #0 event_add (ev=0x1ca403b53a80, tv=0x7f7ffffe0db8) at
> > > /usr/src/lib/libevent/event.c:680
> > > #1 0x00001ca12291949d in hce_launch_checks (fd=-1, event=1,
> > > arg=0x1ca403b52000)
> > > at /usr/src/usr.sbin/relayd/hce.c:191
> > > #2 0x00001ca122919f80 in hce_dispatch_pfe (fd=12, p=0x1ca122b57290,
> > > imsg=0x7f7ffffe0ec8)
> > > at /usr/src/usr.sbin/relayd/hce.c:333
> > > #3 0x00001ca12292247f in proc_dispatch (fd=12, event=2,
> > > arg=0x1ca40d56b000)
> > > at /usr/src/usr.sbin/relayd/proc.c:652
> > > #4 0x00001ca39d264185 in event_base_loop (base=0x1ca391370c00,
> > > flags=Variable "flags" is not available.
> > > )
> > > at /usr/src/lib/libevent/event.c:350
> > > #5 0x00001ca1229231b1 in proc_run (ps=0x1ca3ed565000, p=0x1ca122b57b20,
> > > procs=0x1ca122b57250,
> > > nproc=3, run=0x1ca122918f00 <hce_init>, arg=0x0) at
> > > /usr/src/usr.sbin/relayd/proc.c:594
> > > #6 0x00001ca122918eee in hce (ps=0x1ca3ed565000, p=0x1ca122b57b20)
> > > at /usr/src/usr.sbin/relayd/hce.c:59
> > > #7 0x00001ca122921d0a in proc_init (ps=0x1ca3ed565000,
> > > procs=0x1ca122b57ae0, nproc=4, argc=7,
> > > argv=0x7f7ffffe11f8, proc_id=PROC_HCE) at
> > > /usr/src/usr.sbin/relayd/proc.c:249
> > > #8 0x00001ca122933465 in main (argc=0, argv=0x7f7ffffe11f8)
> > > at /usr/src/usr.sbin/relayd/relayd.c:218
> > >
> > > (gdb) print base
> > > $3 = (struct event_base *) 0x0
> > >
> > > It seems like a (nul) pointer dereference.
> > >
> > > It seems because the table is empty in hce.c hce_setup_events() and the
> > > event
> > > is not initialized:
> > >
> > > if (!(TAILQ_EMPTY(env->sc_tables) ||
> > > event_initialized(&env->sc_ev))) {
> > > evtimer_set(&env->sc_ev, hce_launch_checks, env);
> > > bzero(&tv, sizeof(tv));
> > > evtimer_add(&env->sc_ev, &tv);
> > > }
> > >
> > >
> > > TAILQ_EMPTY(env->sc_tables) is true and the timer is not initialized.
> > > but in hce.c hce_launch_checks() the timer is used:
> > >
> > > evtimer_add(&env->sc_ev, &tv);
> > >
> > >
> > > My test config (/etc/relayd.conf):
> > > table <service> { 127.0.0.1 }
> > >
> > > http protocol "t" {
> > > tcp { nodelay }
> > > }
> > >
> > > relay "r" {
> > > listen on "127.0.0.1" port 80
> > > protocol "t"
> > > forward to <service> port 8080
> > > }
> > >
> > > I hope this helps.
> > >
> >
> > Hi,
> >
> > The below patch makes sure to always initializes the event, even when the
> > table is empty. Other parts of the relayd hce.c code assume the struct event
> > should be initialized aswell.
> >
> > diff --git usr.sbin/relayd/hce.c usr.sbin/relayd/hce.c
> > index 5c5ee6f3013..a67d37f25d8 100644
> > --- usr.sbin/relayd/hce.c
> > +++ usr.sbin/relayd/hce.c
> > @@ -80,8 +80,7 @@ hce_setup_events(void)
> > struct timeval tv;
> > struct table *table;
> >
> > - if (!(TAILQ_EMPTY(env->sc_tables) ||
> > - event_initialized(&env->sc_ev))) {
> > + if (!event_initialized(&env->sc_ev)) {
> > evtimer_set(&env->sc_ev, hce_launch_checks, env);
> > bzero(&tv, sizeof(tv));
> > evtimer_add(&env->sc_ev, &tv);
> >
> > --
> > Kind regards,
> > Hiltjo
> >
--
:wq Claudio
