Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vmod-dynamic accepts port=0 which causes segfault #51

Closed
lukaszlach opened this issue Oct 8, 2019 · 1 comment

Comments

@lukaszlach
Copy link

commented Oct 8, 2019

If you define a service (for example in Consul) and do not provide the port number, SRV records return value of 0. When vmod-dynamic tries to load such backend Varnish crashes with:

Child (24198) Panic at: Tue, 08 Oct 2019 10:45:17 GMT Wrong turn at mgt/mgt_child.c:287: Signal 11 (Segmentation fault) received at 0x10 si_code 1 version = varnish-5.2.0 revision 4c4875cbf, vrt api = 6.1 ident = Linux,3.16.0-6-amd64,x86_64,-junix,-smalloc,-smalloc,-hcritbit,epoll now = 19962215.563448 (mono), 1570531516.494987 (real)

The issue is on this line:
https://github.com/nigoroll/libvmod-dynamic/blob/master/src/dyn_resolver_getdns.c#L386

The internal discussion I have had about this issue ended with conslusion that if the port returned in SRV is invalid (<= 0), vmod should just ignore it and no try to fix it with hardcoded port 80.

@nigoroll nigoroll self-assigned this Oct 9, 2019
@nigoroll

This comment has been minimized.

Copy link
Owner

commented Oct 10, 2019

thank you for the report. The root case was actually different, see fix commit

@nigoroll nigoroll closed this in 8286496 Oct 10, 2019
nigoroll added a commit that referenced this issue Oct 10, 2019
- getservbyname_r can not only fail, but also set the servent pointer
  to NULL for lookup failures. We missed to handle this case

- Following up on this, getdns_fini() also needs to handle error cases,
  for which context and response may be NULL

Fixes #51
nigoroll added a commit that referenced this issue Oct 10, 2019
- getservbyname_r can not only fail, but also set the servent pointer
  to NULL for lookup failures. We missed to handle this case

- Following up on this, getdns_fini() also needs to handle error cases,
  for which context and response may be NULL

Fixes #51
nigoroll added a commit that referenced this issue Oct 10, 2019
- getservbyname_r can not only fail, but also set the servent pointer
  to NULL for lookup failures. We missed to handle this case

- Following up on this, getdns_fini() also needs to handle error cases,
  for which context and response may be NULL

Fixes #51
nigoroll added a commit that referenced this issue Oct 10, 2019
nigoroll added a commit that referenced this issue Oct 10, 2019
nigoroll added a commit that referenced this issue Oct 10, 2019
nigoroll added a commit that referenced this issue Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.