-
Notifications
You must be signed in to change notification settings - Fork 17
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
🏎️ access score impact safely #1526
Conversation
We have a number of errors like this one below, I am not 100% sure why we are accessing an invalid memory address or nil pointer but, this is a best-effort to access the score impact safely. ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x161e309] goroutine 86 [running]: go.mondoo.com/cnspec/v11/policy.(*bandedScoreCalculator).Add(0x22618c0?, 0xc00101f8c0?, 0xc000c83858?) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/score_calculator.go:505 +0x69 go.mondoo.com/cnspec/v11/policy.AddSpecdScore({0x228f728, 0xc001928600}, 0xc000c83858?, 0x58?, 0xc00115fc40) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/score_calculator.go:103 +0xb9 go.mondoo.com/cnspec/v11/policy/executor/internal.(*ReportingJobNodeData).score(0xc0015e8080) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/executor/internal/nodes.go:593 +0x279 go.mondoo.com/cnspec/v11/policy/executor/internal.(*ReportingJobNodeData).recalculate(0xc0015e8080) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/executor/internal/nodes.go:486 +0x3f go.mondoo.com/cnspec/v11/policy/executor/internal.(*GraphExecutor).Execute(0xc000db6370) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/executor/internal/graph.go:121 +0x46d go.mondoo.com/cnspec/v11/policy/executor.ExecuteResolvedPolicy({0x229a8c8, 0xc000343700}, {0x229dcc8, 0xc000e0bec0}, {0xc000e747e0, 0x56}, 0xc001215f00, {0xc000a76160, 0x1, 0x8}, ...) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/executor/graph.go:59 +0x453 go.mondoo.com/cnspec/v11/policy/scan.(*localAssetScanner).runPolicy(0xc0002efd38) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:979 +0x549 go.mondoo.com/cnspec/v11/policy/scan.(*localAssetScanner).run(0xc000c83d38) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:750 +0x33 go.mondoo.com/cnspec/v11/policy/scan.(*LocalScanner).runMotorizedAsset.func1(0xc000d994c0?, 0xc000e0bec0?) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:601 +0x218 go.mondoo.com/cnspec/v11/internal/datalakes/inmemory.WithDb({0x229a8c8?, 0xc000590580?}, 0x3f?, 0xc000e37df8) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/internal/datalakes/inmemory/inmemory.go:52 +0x42 go.mondoo.com/cnspec/v11/policy/scan.(*LocalScanner).runMotorizedAsset(0xc0003951f0?, 0x1fe1566?) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:577 +0x65 go.mondoo.com/cnspec/v11/policy/scan.(*LocalScanner).RunAssetJob(0xc000169080, 0xc000e0c700) /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:524 +0xab go.mondoo.com/cnspec/v11/policy/scan.(*LocalScanner).distributeJob.func3() /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:450 +0x4c5 created by go.mondoo.com/cnspec/v11/policy/scan.(*LocalScanner).distributeJob in goroutine 1 /go/pkg/mod/go.mondoo.com/cnspec/[email protected]/policy/scan/local_scanner.go:416 +0x836 ``` Signed-off-by: Salim Afiune Maya <[email protected]>
Test Results 1 files 27 suites 1m 6s ⏱️ Results for commit 7df4daf. ♻️ This comment has been updated with latest results. |
Signed-off-by: Salim Afiune Maya <[email protected]>
Co-authored-by: Christian Zunker <[email protected]>
201624e
to
7df4daf
Compare
@@ -501,8 +501,13 @@ func (c *bandedScoreCalculator) Add(score *Score, impact *explorer.Impact) { | |||
|
|||
if score.ScoreCompletion != 0 && score.Weight != 0 { | |||
category := uint32(0) | |||
if impact != nil && impact.Value != nil { | |||
category = 100 - uint32(impact.Value.Value) | |||
if impact != nil { |
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.
these 2 look the same to me. Am i missing somethng
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.
i fixed the panic in https://github.com/mondoohq/cnspec/pull/1514/files
@@ -590,7 +590,8 @@ func (nodeData *ReportingJobNodeData) score() (*policy.Score, error) { | |||
if s == nil { | |||
return nil, nil | |||
} | |||
policy.AddSpecdScore(calculator, s, rjRes.score != nil, rjRes.impact) | |||
i := rjRes.impact |
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.
i don't understand why this makes it safer to access
Closing in favor of #1514 |
We have a number of errors like this one below, I am not 100% sure why we are accessing
an invalid memory address or nil pointer since we are indeed checking the pointers are not
nil but, the only thing I can think of is:
int32
touint32
and we are overflowing (not likely but I added a check)Note we did not get notified of this panic because of #1525