Skip to content

strings.Split method panic #56741

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

Closed
beichengcx opened this issue Nov 15, 2022 · 6 comments
Closed

strings.Split method panic #56741

beichengcx opened this issue Nov 15, 2022 · 6 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@beichengcx
Copy link

beichengcx commented Nov 15, 2022

What version of Go are you using (go version)?

$ go version
 go1.17.1 linux/amd64

Does this issue reproduce with the latest release?

No

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/cc/.cache/go-build"
GOENV="/home/cc/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/cc/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/cc/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3342112856=/tmp/go-build -gno-record-gcc-switches"

What did you do?

When calling the go strings.Split method, it sometimes panics.

type ClientInfo struct {
	Langpack       string
	Langcode       string
	ClientIP       string
	CountryCode    string
}
var ip = info.ClientIP
	x := strings.Split(ip, ":")
	if len(x) > 1 {
		ip = x[0]
	}

What did you expect to see?

string sclice

What did you see instead?

 /var/jenkins_home/workspace/server-prod-base/pkg/rpcserver/middleware/server.go:39 +0x15c
panic({0x207a940, 0x3b84350})
 /var/jenkins_home/go/src/runtime/panic.go:1038 +0x215
strings.Count({0x0, 0x0}, {0x2747ca0, 0x46ae47})
 /var/jenkins_home/go/src/strings/strings.go:47 +0x50
strings.genSplit({0x0, 0x14}, {0x2747ca0, 0x1}, 0x0, 0x27ee408)
 /var/jenkins_home/go/src/strings/strings.go:244 +0x52
strings.Split(...)
 /var/jenkins_home/go/src/strings/strings.go:299
git.achat.im/im_session/server.uploadInitConnection(0x0, 0xaf82a571fe57be1d, 0xc085d37d28)
 /var/jenkins_home/workspace/server-prod-session/server/cache_auth_manager.go:231 +0x8d
@beichengcx beichengcx changed the title affected/package: strings.Split method panic Nov 15, 2022
@seankhliao
Copy link
Member

Where does the string come from? constructed with unsafe?
Does the program have a data race?

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2022
@mengzhuo
Copy link
Contributor

mengzhuo commented Nov 15, 2022

strings.Count({0x0, 0x0}, {0x2747ca0, 0x46ae47})
 /var/jenkins_home/go/src/strings/strings.go:47 +0x50

It looks like strings is a null 0x0, could you upload a mininal working example in https://go.dev/play ?

@beichengcx
Copy link
Author

beichengcx commented Nov 15, 2022

Where does the string come from? constructed with unsafe? Does the program have a data race?

func (s *authSessions) getClientInfo() *ClientInfo {
	if s.client.Layer == 0 || s.client.Langpack == "" || s.client.AppVersion == "" {
		v, err := gCacheAuthManager.GetAuthSession(s.authKeyId)
		if err != nil {
			log.Warnf("authSessions - GetAuthSession: %v", err)
		}
		if v != nil {
			client := &ClientInfo{
				ApiId:          v.ApiId,
				DeviceModel:    v.DeviceModel,
				AppVersion:     v.AppVersion,
				SystemVersion:  v.SystemVersion,
				SystemLangcode: v.SystemLangCode,
				Langpack:       v.LangPack,
				Langcode:       v.LangCode,
				Layer:          v.Layer,
			}
			s.setClientInfo(client, false)
		}
	}
	s.clientLocker.RLock()
	defer s.clientLocker.RUnlock()
	client := &ClientInfo{}
	copyClientInfo(client, &s.client)
	return client
}

func (s *authSessions) setClientInfo(v *ClientInfo, upload bool) {
	s.clientLocker.Lock()
	defer s.clientLocker.Unlock()

	var changed bool
	if v.Layer != 0 && s.client.Layer != v.Layer {
		s.client.Layer = v.Layer
		changed = true
	}
	if v.ClientIP != "" && s.client.ClientIP != v.ClientIP {
		s.client.ClientIP = v.ClientIP
		//changed = true
	}
	if v.ApiId != 0 && s.client.ApiId != v.ApiId {
		s.client.ApiId = v.ApiId
		changed = true
	}
	if v.Langcode != "" && s.client.Langcode != v.Langcode {
		s.client.Langcode = v.Langcode
		changed = true
	}
	if v.Langpack != "" && s.client.Langpack != v.Langpack {
		s.client.Langpack = v.Langpack
		changed = true
	}
	if v.AppVersion != "" && s.client.AppVersion != v.AppVersion {
		s.client.AppVersion = v.AppVersion
		changed = true
	}
	if v.SystemVersion != "" && s.client.SystemVersion != v.SystemVersion {
		s.client.SystemVersion = v.SystemVersion
		changed = true
	}
	if v.DeviceModel != "" && s.client.DeviceModel != v.DeviceModel {
		s.client.DeviceModel = v.DeviceModel
		changed = true
	}
	if v.SystemLangcode != "" && s.client.SystemLangcode != v.SystemLangcode {
		s.client.SystemLangcode = v.SystemLangcode
		changed = true
	}
	return
}

func copyClientInfo(dst, src *ClientInfo) {
	dst.Layer = src.Layer
	dst.ApiId = src.ApiId
	dst.DeviceModel = string([]byte(src.DeviceModel))
	dst.AppVersion = string([]byte(src.AppVersion))
	dst.SystemVersion = string([]byte(src.SystemVersion))
	dst.SystemLangcode = string([]byte(src.SystemLangcode))
	dst.Langpack = string([]byte(src.Langpack))
	dst.Langcode = string([]byte(src.Langcode))
	dst.ClientIP = string([]byte(src.ClientIP))
	dst.CountryCode = string([]byte(src.CountryCode))
}

Here is my code to give initial value to ip variable

@randall77
Copy link
Contributor

strings.genSplit({0x0, 0x14}, {0x2747ca0, 0x1}, 0x0, 0x27ee408)

That first string argument looks like it has a nil pointer but a non-zero length. That should never happen.

Please run your code under the race detector. Racy writes are one way such an invalid string might be produced.

I don't know if we're going to be able to help you more without a way to reproduce. The problem is not with strings, it is with whatever process is generating that string.

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2022
@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2022
@mengzhuo
Copy link
Contributor

@randall77 This may help.

I think some deserialize libraries might change []byte type into string just for performance.

package main

import (
	"fmt"
	"reflect"
	"unsafe"
)

var unsafestrptr = *(*string)(unsafe.Pointer(
	&reflect.StringHeader{
		Data: 0,
		Len:  12}))

func main() {
	fmt.Println("Hello, 世界", unsafestrptr)
}

@seankhliao seankhliao added this to the Unplanned milestone Nov 19, 2022
@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants