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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35300a0
bpf: OpenMap do not leak fd
kkourt Jun 17, 2023
a112175
observer_stats: use GetNumPossibleCPUs()
kkourt Jun 17, 2023
e9a2c04
updateMapMetric: close maps on errors
kkourt Jun 17, 2023
fae4ef2
testObserverOptions: remove unused pretty field
kkourt Jun 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/bpf/map_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,17 @@ func OpenMap(name string) (*Map, error) {

info, err := GetMapInfo(os.Getpid(), fd)
if err != nil {
unix.Close(fd)
return nil, err
}

if info.MapType == 0 {
unix.Close(fd)
return nil, fmt.Errorf("Unable to determine map type")
}

if info.KeySize == 0 {
unix.Close(fd)
kkourt marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("Unable to determine map key size")
}

Expand Down
67 changes: 36 additions & 31 deletions pkg/observer/observer_stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (k *statKey) String() string { return fmt.Sprintf("key=%d", k.Ke
func (k *statKey) GetKeyPtr() unsafe.Pointer { return unsafe.Pointer(k) }
func (k *statKey) NewValue() bpf.MapValue {
return &statValue{
Value: make([]int64, runtime.NumCPU()),
Value: make([]int64, bpf.GetNumPossibleCPUs()),
}
}
func (k *statKey) DeepCopyMapKey() bpf.MapKey { return &statKey{k.Key} }
Expand All @@ -41,44 +41,49 @@ func (s *statValue) GetValuePtr() unsafe.Pointer {
}
func (s *statValue) DeepCopyMapValue() bpf.MapValue {
v := &statValue{
Value: make([]int64, runtime.NumCPU()),
Value: make([]int64, len(s.Value)),
}
copy(v.Value, s.Value)
return v
}

func (k *Observer) startUpdateMapMetrics() {
update := func() {
for _, m := range sensors.AllMaps {
pin := filepath.Join(option.Config.MapDir, m.Name)
pinStats := pin + "_stats"
func updateMapMetric(name string) {
pin := filepath.Join(option.Config.MapDir, name)
pinStats := pin + "_stats"

mapLinkStats, err := bpf.OpenMap(pinStats)
if err != nil {
continue
}
mapLink, err := bpf.OpenMap(pin)
if err != nil {
continue
}
mapLinkStats, err := bpf.OpenMap(pinStats)
if err != nil {
return
}
defer mapLinkStats.Close()
mapLink, err := bpf.OpenMap(pin)
if err != nil {
return
}
defer mapLink.Close()

zeroKey := &statKey{}
value, err := mapLinkStats.Lookup(zeroKey)
if err != nil {
continue
}
zeroKey := &statKey{}
value, err := mapLinkStats.Lookup(zeroKey)
if err != nil {
return
}

v, ok := value.DeepCopyMapValue().(*statValue)
if !ok {
continue
}
sum := int64(0)
for cpu := int(0); cpu < runtime.NumCPU(); cpu++ {
sum += v.Value[cpu]
}
mapmetrics.MapSizeSet(m.Name, int(mapLink.MapInfo.MaxEntries), float64(sum))
mapLink.Close()
mapLinkStats.Close()
v, ok := value.DeepCopyMapValue().(*statValue)
kkourt marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
return
}

sum := int64(0)
for cpu := int(0); cpu < runtime.NumCPU(); cpu++ {
sum += v.Value[cpu]
}
mapmetrics.MapSizeSet(name, int(mapLink.MapInfo.MaxEntries), float64(sum))
}

func (k *Observer) startUpdateMapMetrics() {
update := func() {
for _, m := range sensors.AllMaps {
updateMapMetric(m.Name)
}
}

Expand Down
14 changes: 0 additions & 14 deletions pkg/observer/observer_test_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ var (
)

type testObserverOptions struct {
pretty bool
crd bool
config string
lib string
Expand Down Expand Up @@ -97,12 +96,6 @@ func WithDenyList(denyList *tetragon.Filter) TestOption {
}
}

func withPretty() TestOption {
return func(o *TestOptions) {
o.observer.pretty = true
}
}

func WithConfig(config string) TestOption {
return func(o *TestOptions) {
o.observer.config = config
Expand Down Expand Up @@ -198,7 +191,6 @@ func newDefaultTestOptions(opts ...TestOption) *TestOptions {
// default values
options := &TestOptions{
observer: testObserverOptions{
pretty: false,
crd: false,
config: "",
lib: "",
Expand Down Expand Up @@ -323,15 +315,13 @@ func GetDefaultObserverWithWatchers(t *testing.T, ctx context.Context, base *sen

func GetDefaultObserverWithBase(t *testing.T, ctx context.Context, b *sensors.Sensor, file, lib string, opts ...TestOption) (*Observer, error) {
opts = append(opts, WithConfig(file))
opts = append(opts, withPretty())
opts = append(opts, WithLib(lib))

return GetDefaultObserverWithWatchers(t, ctx, b, opts...)
}

func GetDefaultObserverWithFile(t *testing.T, ctx context.Context, file, lib string, opts ...TestOption) (*Observer, error) {
opts = append(opts, WithConfig(file))
opts = append(opts, withPretty())
opts = append(opts, WithLib(lib))

b := base.GetInitialSensor()
Expand All @@ -340,7 +330,6 @@ func GetDefaultObserverWithFile(t *testing.T, ctx context.Context, file, lib str

func GetDefaultSensorsWithFile(t *testing.T, ctx context.Context, file, lib string, opts ...TestOption) ([]*sensors.Sensor, error) {
opts = append(opts, WithConfig(file))
opts = append(opts, withPretty())
opts = append(opts, WithLib(lib))

b := base.GetInitialSensor()
Expand All @@ -350,7 +339,6 @@ func GetDefaultSensorsWithFile(t *testing.T, ctx context.Context, file, lib stri

func GetDefaultObserverWithFileNoTest(t *testing.T, ctx context.Context, file, lib string, fail bool, opts ...TestOption) (*Observer, error) {
opts = append(opts, WithConfig(file))
opts = append(opts, withPretty())
opts = append(opts, WithLib(lib))
opts = append(opts, withNotestfail(fail))

Expand Down Expand Up @@ -589,7 +577,6 @@ func WriteConfigFile(fileName, config string) error {
func GetDefaultObserver(t *testing.T, ctx context.Context, lib string, opts ...TestOption) (*Observer, error) {
b := base.GetInitialSensor()

opts = append(opts, withPretty())
opts = append(opts, WithLib(lib))

return GetDefaultObserverWithWatchers(t, ctx, b, opts...)
Expand All @@ -598,7 +585,6 @@ func GetDefaultObserver(t *testing.T, ctx context.Context, lib string, opts ...T
func GetDefaultObserverWithConfig(t *testing.T, ctx context.Context, config, lib string, opts ...TestOption) (*Observer, error) {
b := base.GetInitialSensor()

opts = append(opts, withPretty())
opts = append(opts, WithConfig(config))
opts = append(opts, WithLib(lib))

Expand Down