Skip to content

pkg/identity: protect LabelsSHA256 against concurrent initializations#11872

Merged
joestringer merged 1 commit intomasterfrom
pr/fix-fqdn-datarace
Jun 4, 2020
Merged

pkg/identity: protect LabelsSHA256 against concurrent initializations#11872
joestringer merged 1 commit intomasterfrom
pr/fix-fqdn-datarace

Conversation

@aanm
Copy link
Copy Markdown
Member

@aanm aanm commented Jun 3, 2020

LabelsSHA256 can be protected with a sync.Once and it makes sure this
field is only initialized once.

Fixes: a77ba96 ("create identity package")
Signed-off-by: André Martins andre@cilium.io

Fixes:

020-06-03T14:40:19.614962016Z ==================
2020-06-03T14:40:19.615012315Z WARNING: DATA RACE
2020-06-03T14:40:19.61501701Z Write at 0x00c00053f190 by goroutine 368:
2020-06-03T14:40:19.61503239Z   github.com/cilium/cilium/pkg/identity.(*Identity).GetLabelsSHA256()
2020-06-03T14:40:19.615034802Z       /go/src/github.com/cilium/cilium/pkg/identity/identity.go:96 +0x125
2020-06-03T14:40:19.61503716Z   github.com/cilium/cilium/pkg/endpoint.(*Endpoint).GetLabelsSHA()
2020-06-03T14:40:19.61503937Z       /go/src/github.com/cilium/cilium/pkg/endpoint/endpoint.go:535 +0x5e
2020-06-03T14:40:19.615041718Z   github.com/cilium/cilium/pkg/endpoint.(*Endpoint).GetProxyInfoByFields()
2020-06-03T14:40:19.615043952Z       /go/src/github.com/cilium/cilium/pkg/endpoint/endpoint.go:2230 +0x257
2020-06-03T14:40:19.615124335Z   github.com/cilium/cilium/pkg/proxy/logger.getEndpointInfo()
2020-06-03T14:40:19.615127828Z       /go/src/github.com/cilium/cilium/pkg/proxy/logger/epinfo.go:52 +0x59
2020-06-03T14:40:19.615130878Z   github.com/cilium/cilium/pkg/proxy/logger.NewLogRecord()
2020-06-03T14:40:19.615133175Z       /go/src/github.com/cilium/cilium/pkg/proxy/logger/logger.go:98 +0x152
2020-06-03T14:40:19.615135452Z   github.com/cilium/cilium/daemon/cmd.(*Daemon).notifyOnDNSMsg()
2020-06-03T14:40:19.615137715Z       /go/src/github.com/cilium/cilium/daemon/cmd/fqdn.go:486 +0xd75
2020-06-03T14:40:19.615152789Z   github.com/cilium/cilium/daemon/cmd.(*Daemon).notifyOnDNSMsg-fm()
2020-06-03T14:40:19.615155497Z       /go/src/github.com/cilium/cilium/daemon/cmd/fqdn.go:408 +0x104
2020-06-03T14:40:19.615157756Z   github.com/cilium/cilium/pkg/fqdn/dnsproxy.(*DNSProxy).ServeDNS()
2020-06-03T14:40:19.615159995Z       /go/src/github.com/cilium/cilium/pkg/fqdn/dnsproxy/proxy.go:434 +0x1cea
2020-06-03T14:40:19.615162291Z   github.com/miekg/dns.(*Server).serveDNS()
2020-06-03T14:40:19.615164499Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:597 +0x3de
2020-06-03T14:40:19.615166845Z   github.com/miekg/dns.(*Server).serveUDPPacket()
2020-06-03T14:40:19.615169066Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:552 +0x21b
2020-06-03T14:40:19.615171317Z   github.com/miekg/dns.(*Server).serveUDP.func2()
2020-06-03T14:40:19.615173507Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:478 +0x78
2020-06-03T14:40:19.615175812Z 
2020-06-03T14:40:19.615178029Z Previous read at 0x00c00053f190 by goroutine 340:
2020-06-03T14:40:19.615180273Z   github.com/cilium/cilium/pkg/identity.(*Identity).GetLabelsSHA256()
2020-06-03T14:40:19.615182514Z       /go/src/github.com/cilium/cilium/pkg/identity/identity.go:95 +0x90
2020-06-03T14:40:19.615433146Z   github.com/cilium/cilium/pkg/endpoint.(*Endpoint).GetLabelsSHA()
2020-06-03T14:40:19.615438192Z       /go/src/github.com/cilium/cilium/pkg/endpoint/endpoint.go:535 +0x5e
2020-06-03T14:40:19.615447077Z   github.com/cilium/cilium/pkg/endpoint.(*Endpoint).GetProxyInfoByFields()
2020-06-03T14:40:19.615450854Z       /go/src/github.com/cilium/cilium/pkg/endpoint/endpoint.go:2230 +0x257
2020-06-03T14:40:19.615453261Z   github.com/cilium/cilium/pkg/proxy/logger.getEndpointInfo()
2020-06-03T14:40:19.615455527Z       /go/src/github.com/cilium/cilium/pkg/proxy/logger/epinfo.go:52 +0x59
2020-06-03T14:40:19.61545779Z   github.com/cilium/cilium/pkg/proxy/logger.NewLogRecord()
2020-06-03T14:40:19.615460101Z       /go/src/github.com/cilium/cilium/pkg/proxy/logger/logger.go:98 +0x152
2020-06-03T14:40:19.615462366Z   github.com/cilium/cilium/daemon/cmd.(*Daemon).notifyOnDNSMsg()
2020-06-03T14:40:19.615478946Z       /go/src/github.com/cilium/cilium/daemon/cmd/fqdn.go:486 +0xd75
2020-06-03T14:40:19.615481278Z   github.com/cilium/cilium/daemon/cmd.(*Daemon).notifyOnDNSMsg-fm()
2020-06-03T14:40:19.615483541Z       /go/src/github.com/cilium/cilium/daemon/cmd/fqdn.go:408 +0x104
2020-06-03T14:40:19.615485787Z   github.com/cilium/cilium/pkg/fqdn/dnsproxy.(*DNSProxy).ServeDNS()
2020-06-03T14:40:19.615488056Z       /go/src/github.com/cilium/cilium/pkg/fqdn/dnsproxy/proxy.go:434 +0x1cea
2020-06-03T14:40:19.61549045Z   github.com/miekg/dns.(*Server).serveDNS()
2020-06-03T14:40:19.61550533Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:597 +0x3de
2020-06-03T14:40:19.615702982Z   github.com/miekg/dns.(*Server).serveUDPPacket()
2020-06-03T14:40:19.615708945Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:552 +0x21b
2020-06-03T14:40:19.615711409Z   github.com/miekg/dns.(*Server).serveUDP.func2()
2020-06-03T14:40:19.615713657Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:478 +0x78
2020-06-03T14:40:19.615715937Z 
2020-06-03T14:40:19.615718133Z Goroutine 368 (running) created at:
2020-06-03T14:40:19.615720421Z   github.com/miekg/dns.(*Server).serveUDP()
2020-06-03T14:40:19.615722671Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:477 +0x31b
2020-06-03T14:40:19.615725005Z   github.com/miekg/dns.(*Server).ActivateAndServe()
2020-06-03T14:40:19.615727233Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:329 +0x3f7
2020-06-03T14:40:19.615729909Z   github.com/cilium/cilium/pkg/fqdn/dnsproxy.StartDNSProxy.func1()
2020-06-03T14:40:19.615732227Z       /go/src/github.com/cilium/cilium/pkg/fqdn/dnsproxy/proxy.go:294 +0xc2
2020-06-03T14:40:19.615734511Z 
2020-06-03T14:40:19.615736717Z Goroutine 340 (running) created at:
2020-06-03T14:40:19.61573893Z   github.com/miekg/dns.(*Server).serveUDP()
2020-06-03T14:40:19.615741155Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:477 +0x31b
2020-06-03T14:40:19.6157435Z   github.com/miekg/dns.(*Server).ActivateAndServe()
2020-06-03T14:40:19.616071453Z       /go/src/github.com/cilium/cilium/vendor/github.com/miekg/dns/server.go:329 +0x3f7
2020-06-03T14:40:19.61607688Z   github.com/cilium/cilium/pkg/fqdn/dnsproxy.StartDNSProxy.func1()
2020-06-03T14:40:19.616079626Z       /go/src/github.com/cilium/cilium/pkg/fqdn/dnsproxy/proxy.go:294 +0xc2

@aanm aanm added release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.8 labels Jun 3, 2020
@aanm aanm requested a review from a team June 3, 2020 18:06
@aanm
Copy link
Copy Markdown
Member Author

aanm commented Jun 3, 2020

test-me-please

@aanm
pkg/identity: protect LabelsSHA256 against concurrent initializations
3958c70 
LabelsSHA256 can be protected with a sync.Once and it makes sure this
field is only initialized once.

Fixes: a77ba96 ("create identity package")
Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm force-pushed the pr/fix-fqdn-datarace branch from 4f4ee22 to 3958c70 Compare June 3, 2020 19:16
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2020

Coverage Status

Coverage decreased (-0.002%) to 36.931% when pulling 3958c70 on pr/fix-fqdn-datarace into 78d99a5 on master.

@aanm aanm added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 3, 2020
@joestringer joestringer merged commit ec385b2 into master Jun 4, 2020
@joestringer joestringer deleted the pr/fix-fqdn-datarace branch June 4, 2020 03:53
@tklauser tklauser mentioned this pull request Jun 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joestringer joestringer joestringer approved these changes

Assignees

No one assigned

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@aanm @coveralls @joestringer @tklauser @borkmann