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?
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
>