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 memory corruption bug #1090

Merged
merged 4 commits into from
Jun 19, 2023
Merged

Fix memory corruption bug #1090

merged 4 commits into from
Jun 19, 2023

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Jun 18, 2023

This PR includes a bug that fixes a memory corruption bug, and it includes some other minor patches as well. Please review patch-by-patch.

Fixes: #1083
Fixes: #193

@kkourt kkourt requested a review from a team as a code owner June 18, 2023 20:47
@kkourt kkourt requested a review from tpapagian June 18, 2023 20:47
kkourt added 4 commits June 19, 2023 11:44
Close fds in case of returning with an error.

Signed-off-by: Kornilios Kourtis <[email protected]>
runtime.NumCPU() returns the number of CPUs available to the process.
This excludes offline CPUs, but also does not take into acount task CPU
affinity.

For example:

```
$ cat numcpus.go
package main

import (
        "fmt"
        "runtime"
)

func main() {
        fmt.Printf("%d\n", runtime.NumCPU())
}
$ go run numcpus.go
16
$ taskset --cpu-list 0 go run numcpus.go
1
```

This leads to memory corruption because we pass to the bpf() call a
buffer smaller than the write it will perform, leading to data being
overwritten. Reducing the timeout of the goroutine that reads stat maps
will reproduce this quickly and lead to SIGSEGVs, corrupted prometheus
metrics or other symptoms.

Signed-off-by: Kornilios Kourtis <[email protected]>
create updateMapMetric() function to update the metrics for a single
map. This allows to use defer() to do proper cleanup in case of an error.

Signed-off-by: Kornilios Kourtis <[email protected]>
@kkourt kkourt force-pushed the pr/kkourt/memory-corruption-fix branch from 9baecf3 to fae4ef2 Compare June 19, 2023 09:44
@kkourt kkourt added kind/backport This PR provides functionality previously merged into master. needs-backport/0.8 and removed kind/backport This PR provides functionality previously merged into master. labels Jun 19, 2023
Copy link
Contributor

@kevsecurity kevsecurity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One comment unrelated, so feel free to ignore for now.

@kkourt kkourt merged commit 8f19ada into main Jun 19, 2023
@kkourt kkourt deleted the pr/kkourt/memory-corruption-fix branch June 19, 2023 11:01
This was referenced Jun 27, 2023
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.

Null bytes in process binary names NULLs in tetragon data
3 participants