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.

Sign up for GitHub

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

Jump to bottom

net: high cpu usage in extractExtendedRCode #66754

Closed
long-name-let-people-remember-you opened this issue Apr 10, 2024 · 9 comments
Closed

net: high cpu usage in extractExtendedRCode #66754

long-name-let-people-remember-you opened this issue Apr 10, 2024 · 9 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. Security
Milestone
Go1.23

Comments

@long-name-let-people-remember-you
Copy link

long-name-let-people-remember-you commented Apr 10, 2024 •
edited
Loading

Go version

go 1.22.2 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/usr/local/go/bin'
GOCACHE='/home/xx/.cache/go-build'
GOENV='/home/xx/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/xx/gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/xx/gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/xx/gopath/dns/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build185933323=/tmp/go-build -gno-record-gcc-switches'

What did you do?

concurrent calls to net.LookupAddr

What did you see happen?

pprof seems that goLookupIp may causes high cpu usage
image
image
image
profile.zip

What did you expect to see?

Occasionally, I don’t know what domain name it was when it happened.
@ianlancetaylor ianlancetaylor changed the title net/dnsclient_unix.go: may cause high cpu usage net: high cpu usage in dnsclient_unix.go Apr 10, 2024
@ianlancetaylor
Copy link
Member

This code is processing DNS packets. It would help to know what DNS names are being looked up.

That said, at least part of the problem is that methods like dnsmessage.Parser.AdditionalHeader return a value of type ResourceHeader which is 268 bytes on amd64. That's why so much time is being spent in duffcopy.

Still, it should only be an issue if the DNS server is returning a lot of additional headers. It's not clear why that would be happening.

We started looking at the additional headers in https://go.dev/cl/514835. CC @mateusz834

@mateusz834
Copy link
Member

Yep, this looks like a DNS response with a bunch of Additional resources, it would be helpful if you could provide us with the raw DNS message reply.

@mateusz834 mateusz834 changed the title net: high cpu usage in dnsclient_unix.go net: high cpu usage in extractExtendedRCode Apr 10, 2024
@cagedmantis
Copy link
Contributor

@neild

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 11, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Apr 11, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578375 mentions this issue: net: check SkipAdditional error result

gopherbot pushed a commit that referenced this issue Apr 11, 2024
@ianlancetaylor @gopherbot
net: check SkipAdditional error result
ddfab21 
This will avoid a potential endless loop for a corrupt DNS packet.

For #66754

Change-Id: I46591b3f7695bcc88d2312833e45955f8c129d2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/578375
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
@rolandshoemaker
Copy link
Member

@gopherbot please open backport issues, this is a PUBLIC track security issue.

@gopherbot gopherbot mentioned this issue Apr 25, 2024
Closed
@gopherbot
Copy link
Contributor

gopherbot commented Apr 25, 2024 •
edited by dmitshur
Loading

Backport issue(s) opened: #67039 (for 1.21), #67040 (for 1.22).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Apr 25, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/581816 mentions this issue: [release-branch.go1.22] net: check SkipAdditional error result

@gopherbot

This comment was marked as resolved.

Sign in to view
gopherbot pushed a commit that referenced this issue May 1, 2024
@ianlancetaylor @dr2chase
[release-branch.go1.22] net: check SkipAdditional error result
93d8777 
This will avoid a potential endless loop for a corrupt DNS packet.

For #66754
Fixes #67040

Change-Id: I46591b3f7695bcc88d2312833e45955f8c129d2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/578375
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit ddfab21)
Reviewed-on: https://go-review.googlesource.com/c/go/+/581816
Reviewed-by: David Chase <drchase@google.com>
@cherrymui
Copy link
Member

CL submitted. Closing. Thanks.

@dmitshur dmitshur modified the milestones: Backlog, Go1.23 May 7, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 7, 2024
mjl- added a commit to mjl-/adns that referenced this issue May 9, 2024
@mjl-
from Go stdlib: fix endless loop on certain corrupt dns packets
2dc8715 
see golang/go#66754
released with go1.22.3.

also check error for new SkipAdditional call related to extended dns errors.
@MisterMX MisterMX mentioned this issue May 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done. Security
Projects
None yet
Milestone
Go1.23
Development

No branches or pull requests
8 participants
@cagedmantis @dmitshur @ianlancetaylor @rolandshoemaker @gopherbot @cherrymui @mateusz834 @long-name-let-people-remember-you