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

Fix runtime panic when config file is empty #99

Merged

Conversation

Furisto
Copy link
Contributor

@Furisto Furisto commented May 28, 2022

When the config file is not specified a runtime panic occcured, as
we tried to dereference the nil config object.

Signed-off-by: Thomas Schubart [email protected]

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

goroutine 1 [running]:
main.hubbleTETRAGONExecute()
	/go/src/github.com/cilium/tetragon/cmd/tetragon/main.go:208 +0xa0a
main.execute.func1(0xc00011a780, {0x1d78f14, 0x1, 0x1})
	/go/src/github.com/cilium/tetragon/cmd/tetragon/main.go:314 +0x8a
github.com/spf13/cobra.(*Command).execute(0xc00011a780, {0xc000132010, 0x1, 0x1})
	/go/src/github.com/cilium/tetragon/vendor/github.com/spf13/cobra/command.go:856 +0x5f8
github.com/spf13/cobra.(*Command).ExecuteC(0xc00011a780)
	/go/src/github.com/cilium/tetragon/vendor/github.com/spf13/cobra/command.go:960 +0x3ad
github.com/spf13/cobra.(*Command).Execute(...)
	/go/src/github.com/cilium/tetragon/vendor/github.com/spf13/cobra/command.go:897
main.execute()
	/go/src/github.com/cilium/tetragon/cmd/tetragon/main.go:394 +0x9be
main.main()
	/go/src/github.com/cilium/tetragon/cmd/tetragon/tetragon.go:11 +0x19ru

When the config file is not specified a runtime panic occcured, as
we tried to dereference the nil config object.

Signed-off-by: Thomas Schubart <[email protected]>
@Furisto Furisto requested a review from a team as a code owner May 28, 2022 17:03
@jrfastab
Copy link
Contributor

jrfastab commented May 29, 2022

@Furisto thanks I introduced this and also just hit it and had the same idea for a fix. So lets ship it.

@willfindlay Its valid to have a no config file, I use this for getting just the exec tracing running in some clusters. If we return an error from readConfig then we would need to capture the error and skip the next step or more such steps. Anyways I think this is reasonably clean as is. So I'm going to merge it. Feel free to continue the debate though if you have strong opinions.

@jrfastab jrfastab merged commit 8d4fb4c into cilium:main May 29, 2022
@willfindlay
Copy link
Contributor

@jrfastab yeah I realized that right after I left my comment. I still think there's probably a way to refactor this to be more idiomatic, but that can be done as follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants