relayd: child sigsegv on relayctl poll cmd.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

relayd: child sigsegv on relayctl poll cmd.

Karel Gardas
Hello,

tested on 6.2 and on 6.2-current. relayctl poll command invoked even
as ordinary user may segfault one of relayd child processes hence
failing whole relayd.

The process which fails is "relayd: hce (relayd)" and it fails with
following crash backtrace:

Program received signal SIGSEGV, Segmentation fault.
event_add (ev=0x902942c7a80, tv=0x7f7ffffda378) at
/usr/src/lib/libevent/event.c:680
680             const struct eventop *evsel = base->evsel;
Current language:  auto; currently minimal
(gdb) where
#0  event_add (ev=0x902942c7a80, tv=0x7f7ffffda378) at
/usr/src/lib/libevent/event.c:680
#1  0x0000090067e11c80 in ?? () from /usr/sbin/relayd
#2  0x0000090067e1222b in ?? () from /usr/sbin/relayd
#3  0x0000090067e17a7f in ?? () from /usr/sbin/relayd
#4  0x000009035656ffac in event_base_loop (base=0x9026e20ec00,
flags=0) at /usr/src/lib/libevent/event.c:350
#5  0x0000090067e183a8 in ?? () from /usr/sbin/relayd
#6  0x0000090067e1742f in ?? () from /usr/sbin/relayd
#7  0x0000090067e23455 in relay_udp_server () from /usr/sbin/relayd
#8  0x0000090067e01374 in ?? () from /usr/sbin/relayd
#9  0x0000000000000000 in ?? ()
(gdb)


The process to duplicate this issue is:
- start relayd as root with -dvv
- ps uxa|grep relayd|grep hce
- gdb /usr/sbin/relayd <process id above>

on another console:
- relayctl poll

In gdb, you should see the crash.

My /etc/relayd.conf:

log updates

http protocol "https" {
                return error

                   match header set "X-Forwarded-For" \
                           value "$REMOTE_ADDR"
                   match header set "X-Forwarded-By" \
                           value "$SERVER_ADDR:$SERVER_PORT"
                   match header set "Keep-Alive" value "$TIMEOUT"

                   match header set "X-Forwarded-Proto" value "https"

                match header append "Connection" value "close"

                   pass

                tls { tlsv1, sslv3, no cipher-server-preference }
}

relay https_nexus {
        listen on fujitsu.localdomain port 9091 tls
        protocol "https"
        forward to localhost port 8081
}

relay https_nexus_docker {
        listen on fujitsu.localdomain port 18444 tls
        protocol "https"
        forward to localhost port 8077
}


My dmesg:

OpenBSD 6.2-current (GENERIC.MP) #146: Fri Oct 13 00:44:27 MDT 2017
    [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8522489856 (8127MB)
avail mem = 8257339392 (7874MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xdee83018 (52 entries)
bios0: vendor FUJITSU // American Megatrends Inc. version "V4.6.5.4
R2.10.0 for D3239-A1x" date 10/24/2014
bios0: FUJITSU PRIMERGY TX1320 M1
acpi0 at bios0: rev 2
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FPDT SSDT SSDT MCFG PRAD HPET SSDT SSDT
BDAT SPMI SPCR DMAR EINJ ERST HEST BERT
acpi0: wakeup devices PXSX(S4) RP01(S4) PXSX(S4) PXSX(S4) RP03(S4)
PXSX(S4) GLAN(S4) EHC1(S4) EHC2(S4) XHC_(S4) HDEF(S4) PEG0(S4)
PEGP(S4) PEG1(S4)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E3-1220 v3 @ 3.10GHz, 3093.41 MHz
cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
acpitimer0: recalibrated TSC frequency 3092847441 Hz
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Xeon(R) CPU E3-1220 v3 @ 3.10GHz, 3092.85 MHz
cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Xeon(R) CPU E3-1220 v3 @ 3.10GHz, 3092.85 MHz
cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Xeon(R) CPU E3-1220 v3 @ 3.10GHz, 3092.85 MHz
cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SENSOR,ARAT
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
ioapic0 at mainbus0: apid 8 pa 0xfec00000, version 20, 24 pins
acpimcfg0 at acpi0 addr 0xf8000000, bus 0-63
acpihpet0 at acpi0: 14318179 Hz
acpihpet0: recalibrated TSC frequency 3092675799 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (P0P2)
acpiprt2 at acpi0: bus -1 (P0PA)
acpiprt3 at acpi0: bus 1 (RP01)
acpiprt4 at acpi0: bus 2 (RP03)
acpiprt5 at acpi0: bus -1 (PEG0)
acpiprt6 at acpi0: bus -1 (PEG1)
acpiec0 at acpi0: not present
acpicpu0 at acpi0: C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS
acpicpu1 at acpi0: C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS
acpicpu2 at acpi0: C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS
acpicpu3 at acpi0: C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS
acpipwrres0 at acpi0: FN00, resource for FAN0
acpipwrres1 at acpi0: FN01, resource for FAN1
acpipwrres2 at acpi0: FN02, resource for FAN2
acpipwrres3 at acpi0: FN03, resource for FAN3
acpipwrres4 at acpi0: FN04, resource for FAN4
acpitz0 at acpi0: critical temperature is 105 degC
acpitz1 at acpi0: critical temperature is 105 degC
"INT3F0D" at acpi0 not configured
"IPI0001" at acpi0 not configured
"PNP0A05" at acpi0 not configured
acpiac0 at acpi0: AC unit online
"ACPI000D" at acpi0 not configured
acpibtn0 at acpi0: PWRB
"PNP0C0B" at acpi0 not configured
"PNP0C0B" at acpi0 not configured
"PNP0C0B" at acpi0 not configured
"PNP0C0B" at acpi0 not configured
"PNP0C0B" at acpi0 not configured
acpivideo0 at acpi0: GFX0
acpivout0 at acpivideo0: DD1F
ipmi at mainbus0 not configured
cpu0: Enhanced SpeedStep 3093 MHz: speeds: 3101, 3100, 2900, 2800,
2600, 2400, 2300, 2100, 1900, 1800, 1600, 1500, 1300, 1100, 1000, 800
MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel Xeon E3-1200 v3 Host" rev 0x06
xhci0 at pci0 dev 20 function 0 "Intel 8 Series xHCI" rev 0x05: msi
usb0 at xhci0: USB revision 3.0
uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub" rev
3.00/1.00 addr 1
em0 at pci0 dev 25 function 0 "Intel I217-LM" rev 0x05: msi, address
90:1b:0e:47:01:61
ehci0 at pci0 dev 26 function 0 "Intel 8 Series USB" rev 0x05: apic 8 int 16
usb1 at ehci0: USB revision 2.0
uhub1 at usb1 configuration 1 interface 0 "Intel EHCI root hub" rev
2.00/1.00 addr 1
ppb0 at pci0 dev 28 function 0 "Intel 8 Series PCIE" rev 0xd5: msi
pci1 at ppb0 bus 1
vga1 at pci1 dev 0 function 0 "Matrox MGA G200e" rev 0x05
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
"ServerEngines iRMC" rev 0x00 at pci1 dev 0 function 1 not configured
ppb1 at pci0 dev 28 function 2 "Intel 8 Series PCIE" rev 0xd5: msi
pci2 at ppb1 bus 2
em1 at pci2 dev 0 function 0 "Intel I210" rev 0x03: msi, address
90:1b:0e:44:43:35
ehci1 at pci0 dev 29 function 0 "Intel 8 Series USB" rev 0x05: apic 8 int 23
usb2 at ehci1: USB revision 2.0
uhub2 at usb2 configuration 1 interface 0 "Intel EHCI root hub" rev
2.00/1.00 addr 1
pcib0 at pci0 dev 31 function 0 "Intel C224 LPC" rev 0x05
ahci0 at pci0 dev 31 function 2 "Intel 8 Series AHCI" rev 0x05: msi, AHCI 1.3
ahci0: port 0: 1.5Gb/s
ahci0: port 1: 3.0Gb/s
ahci0: port 2: 3.0Gb/s
ahci0: port 3: 3.0Gb/s
ahci0: port 4: 3.0Gb/s
ahci0: port 5: 3.0Gb/s
scsibus1 at ahci0: 32 targets
cd0 at scsibus1 targ 0 lun 0: <HL-DT-ST, DVDRAM GTB0N, QF01> ATAPI
5/cdrom removable
sd0 at scsibus1 targ 1 lun 0: <ATA, Hitachi HTS72505, PC4O> SCSI3
0/direct fixed naa.5000cca5b5dac689
sd0: 476940MB, 512 bytes/sector, 976773168 sectors
sd1 at scsibus1 targ 2 lun 0: <ATA, WDC WD7500BPKT-7, 01.0> SCSI3
0/direct fixed naa.50014ee603669123
sd1: 715404MB, 512 bytes/sector, 1465149168 sectors
sd2 at scsibus1 targ 3 lun 0: <ATA, Hitachi HTS72505, PC4O> SCSI3
0/direct fixed naa.5000cca5b5dac0a4
sd2: 476940MB, 512 bytes/sector, 976773168 sectors
sd3 at scsibus1 targ 4 lun 0: <ATA, TOSHIBA HDWD130, MX6O> SCSI3
0/direct fixed naa.5000039fe6c3eff5
sd3: 2861588MB, 512 bytes/sector, 5860533168 sectors
sd4 at scsibus1 targ 5 lun 0: <ATA, TOSHIBA HDWD130, MX6O> SCSI3
0/direct fixed naa.5000039fe6c3f742
sd4: 2861588MB, 512 bytes/sector, 5860533168 sectors
ichiic0 at pci0 dev 31 function 3 "Intel 8 Series SMBus" rev 0x05: apic 8 int 18
iic0 at ichiic0
sdtemp0 at iic0 addr 0x19: stts2002
spdmem0 at iic0 addr 0x51: 8GB DDR3 SDRAM ECC PC3-12800 with thermal sensor
"Intel 8 Series Thermal" rev 0x05 at pci0 dev 31 function 6 not configured
isa0 at pcib0
isadma0 at isa0
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
com0: console
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
vmm0 at mainbus0: VMX/EPT
uhidev0 at uhub0 port 5 configuration 1 interface 0 "American
Megatrends Inc. Virtual Keyboard and Mouse" rev 1.10/1.00 addr 2
uhidev0: iclass 3/1
ukbd0 at uhidev0: 8 variable keys, 6 key codes
wskbd0 at ukbd0: console keyboard, using wsdisplay0
uhidev1 at uhub0 port 5 configuration 1 interface 1 "American
Megatrends Inc. Virtual Keyboard and Mouse" rev 1.10/1.00 addr 2
uhidev1: iclass 3/1
ums0 at uhidev1: 3 buttons, Z dir
wsmouse0 at ums0 mux 0
uhub3 at uhub1 port 1 configuration 1 interface 0 "Intel Rate Matching
Hub" rev 2.00/0.05 addr 2
uhub4 at uhub2 port 1 configuration 1 interface 0 "Intel Rate Matching
Hub" rev 2.00/0.05 addr 2
vscsi0 at root
scsibus2 at vscsi0: 256 targets
softraid0 at root
scsibus3 at softraid0: 256 targets
sd5 at scsibus3 targ 1 lun 0: <OPENBSD, SR RAID 1, 006> SCSI2 0/direct fixed
sd5: 476939MB, 512 bytes/sector, 976772640 sectors
root on sd1a (d922ff184e32e614.a) swap on sd1b dump on sd1b

Thanks for looking into this,

Karel

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Hiltjo Posthuma
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.

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Hiltjo Posthuma
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

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Hiltjo Posthuma
On Sat, Nov 18, 2017 at 05:06:33PM +0100, Hiltjo Posthuma wrote:

> 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);
>

*bump*

So... any OKs? :)

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Hiltjo Posthuma
On Mon, Nov 27, 2017 at 06:32:04PM +0100, Hiltjo Posthuma wrote:

> On Sat, Nov 18, 2017 at 05:06:33PM +0100, Hiltjo Posthuma wrote:
> > 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);
> >
>
> *bump*
>
> So... any OKs? :)
>

Weekly *bump*. A reply would be appreciated.

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Sebastian Benoit-3
In reply to this post by Hiltjo Posthuma

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([hidden email]) 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
>

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Claudio Jeker-3
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([hidden email]) 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

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Sebastian Benoit-3
Claudio Jeker([hidden email]) on 2017.12.10 12:52:55 +0100:

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

I thought you would catch that ;)

Lets just run the timer unconditionaly.

ok?


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);

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Claudio Jeker-3
On Sat, Dec 16, 2017 at 10:44:52PM +0100, Sebastian Benoit wrote:

> Claudio Jeker([hidden email]) on 2017.12.10 12:52:55 +0100:
> > 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.
>
> I thought you would catch that ;)
>
> Lets just run the timer unconditionaly.
>
> ok?
>
>
> 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);
>

OK claudio@

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: relayd: child sigsegv on relayctl poll cmd.

Sebastian Benoit-3
commited, thanks for your fix and bug reports.

/Benno

Claudio Jeker([hidden email]) on 2017.12.17 12:01:42 +0100:

> On Sat, Dec 16, 2017 at 10:44:52PM +0100, Sebastian Benoit wrote:
> > Claudio Jeker([hidden email]) on 2017.12.10 12:52:55 +0100:
> > > 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.
> >
> > I thought you would catch that ;)
> >
> > Lets just run the timer unconditionaly.
> >
> > ok?
> >
> >
> > 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);
> >
>
> OK claudio@
>
> --
> :wq Claudio
>