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

service: Skip upsert of service for disabled IP family #15026

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

pchaigno
Copy link
Member

The following panic can happen when trying to upsert e.g. an IPv6 service in the datapath when IPv6 is disabled in the agent. The corresponding IPv6 lbmap doesn't exist so we get a nil pointer reference.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1cd5900]

goroutine 147 [running]:
github.com/cilium/cilium/pkg/bpf.(*Map).OpenOrCreate(0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/cilium/cilium/pkg/bpf/map_linux.go:464 +0x40
github.com/cilium/cilium/pkg/maps/lbmap.updateRevNatLocked(0x2b1b5a0, 0xc000b365fc, 0x2b07380, 0xc000ae7e80, 0xc000b365f0, 0x1)
        /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:331 +0x68
github.com/cilium/cilium/pkg/maps/lbmap.(*LBBPFMap).UpsertService(0xc0009f5040, 0xc000b3a1e0, 0x0, 0xc000b47470)
        /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:130 +0x5b7
github.com/cilium/cilium/pkg/service.(*Service).upsertServiceIntoLBMaps(0xc00065a280, 0xc00066b420, 0x421e500, 0x0, 0x421e540, 0x0, 0x0, 0x421e540, 0x0, 0x0, ...)
        /go/src/github.com/cilium/cilium/pkg/service/service.go:749 +0x3de
github.com/cilium/cilium/pkg/service.(*Service).UpsertService(0xc00065a280, 0xc000661a40, 0x0, 0x0, 0x0)
        /go/src/github.com/cilium/cilium/pkg/service/service.go:324 +0xc85
github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).addK8sSVCs(0xc000974480, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0x0, 0xc000b22f30, 0xc00012be48, 0x10, 0x7ffff7fb9108)
        /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:743 +0x47b
github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler.func1(0x0, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0xc000b22f30, 0x0, 0xc00012be48, 0xc0009b0390)
        /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:447 +0xbc7
github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler(0xc000974480)
        /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:490 +0x95
created by github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).RunK8sServiceHandler
        /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:495 +0x3f

This commit fixes it by exiting early from UpsertService if trying to upsert a service for an IP family that is disabled in the agent.

Fixes: #15000
Fixes: #14607

@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. needs-backport/1.9 labels Feb 18, 2021
@pchaigno pchaigno requested review from a team and aditighag February 18, 2021 13:55
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Feb 18, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.5 Feb 18, 2021
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

@pchaigno pchaigno added the dont-merge/blocked Another PR must be merged before this one. label Feb 18, 2021
@pchaigno
Copy link
Member Author

I need to add a unit test before merging.

Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@joestringer
Copy link
Member

Marking as "draft" while you prepare the testing side. When you would like additional review, scroll to the bottom of the page and click the "Ready for review" button.

@joestringer joestringer marked this pull request as draft February 24, 2021 22:28
@pchaigno pchaigno removed the dont-merge/blocked Another PR must be merged before this one. label Mar 1, 2021
The following panic can happen when trying to upsert e.g. an IPv6
service in the datapath when IPv6 is disabled in the agent. The
corresponding IPv6 lbmap doesn't exist so we get a nil pointer
reference.

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1cd5900]

    goroutine 147 [running]:
    github.com/cilium/cilium/pkg/bpf.(*Map).OpenOrCreate(0x0, 0x0, 0x0, 0x0)
            /go/src/github.com/cilium/cilium/pkg/bpf/map_linux.go:464 +0x40
    github.com/cilium/cilium/pkg/maps/lbmap.updateRevNatLocked(0x2b1b5a0, 0xc000b365fc, 0x2b07380, 0xc000ae7e80, 0xc000b365f0, 0x1)
            /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:331 +0x68
    github.com/cilium/cilium/pkg/maps/lbmap.(*LBBPFMap).UpsertService(0xc0009f5040, 0xc000b3a1e0, 0x0, 0xc000b47470)
            /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:130 +0x5b7
    github.com/cilium/cilium/pkg/service.(*Service).upsertServiceIntoLBMaps(0xc00065a280, 0xc00066b420, 0x421e500, 0x0, 0x421e540, 0x0, 0x0, 0x421e540, 0x0, 0x0, ...)
            /go/src/github.com/cilium/cilium/pkg/service/service.go:749 +0x3de
    github.com/cilium/cilium/pkg/service.(*Service).UpsertService(0xc00065a280, 0xc000661a40, 0x0, 0x0, 0x0)
            /go/src/github.com/cilium/cilium/pkg/service/service.go:324 +0xc85
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).addK8sSVCs(0xc000974480, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0x0, 0xc000b22f30, 0xc00012be48, 0x10, 0x7ffff7fb9108)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:743 +0x47b
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler.func1(0x0, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0xc000b22f30, 0x0, 0xc00012be48, 0xc0009b0390)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:447 +0xbc7
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler(0xc000974480)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:490 +0x95
    created by github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).RunK8sServiceHandler
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:495 +0x3f

This commit fixes it by exiting early from UpsertService if trying to
upsert a service for an IP family that is disabled in the agent.

Fixes: cilium#15000
Fixes: cilium#14607
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno marked this pull request as ready for review March 1, 2021 13:47
@pchaigno pchaigno requested review from a team and michi-covalent and removed request for a team March 1, 2021 13:47
@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 4, 2021
@aanm aanm merged commit ad61343 into cilium:master Mar 4, 2021
1.10.0 automation moved this from In progress to Done Mar 4, 2021
@pchaigno pchaigno deleted the fix-ipv6-svc-panic branch March 4, 2021 10:33
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.5 Mar 8, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 8, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.5 Mar 8, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.5 Mar 8, 2021
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. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.9.5
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

lbmap: Segfault when disabling IPv6 after adding IPv6 services
9 participants