-
Notifications
You must be signed in to change notification settings - Fork 374
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
pkg: fix versionStrings out of bound #1115
Conversation
4b0df17
to
8a7956c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for taking the time to submit your patch! It indeed fixes the issue.
However, the code is a little bit harder to understand: an error in the os.ReadFile
implies that the len(versionStrings) == 0
will trigger the "error path" with the else statement. But it's correct! Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch!
Could you please fix the static warning?
You can just run goimports -w pkg/kernels/kernels.go
and then fold the change into your patch, and push again.
I've run it on my machine, and this is the resulting diff (it's just a whitespace change) if you want to do it manualy:
index 1e689712..2df7c419 100644
--- a/pkg/kernels/kernels.go
+++ b/pkg/kernels/kernels.go
@@ -65,7 +65,7 @@ func GetKernelVersion(kernelVersion, procfs string) (int, string, error) {
if versionSig, err := os.ReadFile(procfs + "/version_signature"); err == nil {
versionStrings = strings.Fields(string(versionSig))
}
-
+
if len(versionStrings) > 0 {
version = int(KernelStringToNumeric(versionStrings[len(versionStrings)-1]))
verStr = versionStrings[len(versionStrings)-1]
add check to length of versionStrings Signed-off-by: Li Chen <chenli3792@gmail.com>
8a7956c
to
93c3488
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Thanks a lot @chenliTW |
Thanks |
add check to length of versionStrings
fix: #1086