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

fqdn: Fix panic on MarshalJSON() #12218

Merged
merged 1 commit into from Jun 22, 2020

Conversation

pchaigno
Copy link
Member

We need to hold a lock on DNSZombieMappings when calling json.Marshal(), otherwise we risk hitting panics on concurrent writes.

  panic: reflect: call of reflect.Value.IsNil on zero Value [recovered]
   goroutine 2807 [running]:
   ...
   panic(...)
   	/usr/local/go/src/runtime/panic.go:679 +0x1b2
   reflect.Value.IsNil(...)
   	/usr/local/go/src/reflect/value.go:1073
   ...
   github.com/cilium/cilium/pkg/fqdn.(*DNSZombieMappings).MarshalJSON(...)
   	/go/src/github.com/cilium/cilium/pkg/fqdn/cache.go:949 +0x3b

Fixes: f629372 ("fqdn: Add and use DNSZombieMappings in Endpoint")

@pchaigno pchaigno added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Jun 22, 2020
@pchaigno pchaigno requested review from tklauser and a team June 22, 2020 09:36
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Jun 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.6 Jun 22, 2020
@aanm
Copy link
Member

aanm commented Jun 22, 2020

test-me-please

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 22, 2020
We need to hold a lock on DNSZombieMappings when calling json.Marshal(),
otherwise we risk hitting panics on concurrent writes.

  panic: reflect: call of reflect.Value.IsNil on zero Value [recovered]
   goroutine 2807 [running]:
   ...
   panic(...)
   	/usr/local/go/src/runtime/panic.go:679 +0x1b2
   reflect.Value.IsNil(...)
   	/usr/local/go/src/reflect/value.go:1073
   ...
   github.com/cilium/cilium/pkg/fqdn.(*DNSZombieMappings).MarshalJSON(...)
   	/go/src/github.com/cilium/cilium/pkg/fqdn/cache.go:949 +0x3b

Fixes: f629372 ("fqdn: Add and use DNSZombieMappings in Endpoint")
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the pr/pchaigno/fix-lock-dnszombiemappings-marshaljson branch from 50eefb5 to 65c15cb Compare June 22, 2020 09:49
@pchaigno
Copy link
Member Author

test-me-please

@coveralls
Copy link

coveralls commented Jun 22, 2020

Coverage Status

Coverage increased (+0.03%) to 37.161% when pulling 65c15cb on pr/pchaigno/fix-lock-dnszombiemappings-marshaljson into 6501817 on master.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 22, 2020
@aanm aanm merged commit 486bedf into master Jun 22, 2020
1.8.0 automation moved this from In progress to Merged Jun 22, 2020
@aanm aanm deleted the pr/pchaigno/fix-lock-dnszombiemappings-marshaljson branch June 22, 2020 13:01
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.6 Jun 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.7 to Backport done to v1.7 in 1.7.6 Jun 22, 2020
This was referenced Jun 24, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 24, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.0 Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.7.6
Backport done to v1.7
1.8.0
  
Merged
1.8.0
Backport done to v1.8
Development

Successfully merging this pull request may close these issues.

None yet

6 participants