Write to outer map fails due to no wrap around of version in snapshot

[subkin13]

The code in func (s *snapshots) Store(ps *Policies) does not wrap s.lastVersion when it reaches maxSnapshots which eventually causes failure when writing to an outer map - see func (ps *Policies) createNewFilterMapsVersion(bpfModule *bpf.Module) error

Also the following code sets the minimal version to 1, it is never zero, so there is never write to cell #0 in the outer map. Is that on purpose?

s.lastVersion++ // new version
if s.lastVersion >= uint16(MaxPolicies) {
     logger.Warnw("Policies version has wrapped around, resetting to 1")
     s.lastVersion++
}

Also - prune func(*Policies) []error is not initialized so the inner maps are never closed which will eventually cause fd leak

[yanivagman]

@subkin13 thanks for reporting,
@geyslan can you have a look?

[subkin13]

also:

1. in createNewInnerMap - GetBTFFDByID creates another fd for the process, you should close it before exiting the function
2. in createNewFilterMapsVersion - if the call to updateOuterMap fails you should close the newInnerMap fd

[geyslan]

Hi @subkin13, thanks for bringing us your findings.

The code in func (s *snapshots) Store(ps *Policies) does not wrap s.lastVersion when it reaches maxSnapshots...

The Snapshots is a circular buffer of size maxSnapshots. This check

tracee/pkg/policy/snapshots.go

Lines 75 to 79 in a458fa9

	s.lastVersion++ // new version
	if s.lastVersion == 0 {
	logger.Warnw("Policies version has wrapped around, resetting to 1")
	s.lastVersion++
	}

is only to deal with uint wrap around.

The version can and should be increased to the holder's limit, as it communicates the version number of these policies, even if we only have maxSnaphosts at a time. So for this I don't see any problem.

---

Also - prune func(*Policies) []error is not initialized so the inner maps are never closed which will eventually cause fd leak

This is true. I was tackling it in currently closed PRs. That will be solved soon.

i.e.: https://github.com/aquasecurity/tracee/pull/3912/files#diff-aab7c4298207977c2b1b634dfc0fc67eb9004e74324ab1c0df0306869d1714b5R607-R609

in createNewInnerMap - GetBTFFDByID creates another fd for the process, you should close it before exiting the function

I suppose you're right. I need to check if after the map creation the BTFFD is not required anymore. Let's deal with it in upcoming efforts. Thanks!

in createNewFilterMapsVersion - if the call to updateOuterMap fails you should close the newInnerMap fd

This is also true and was being tackled on the referred PR.

Gonna close this and update #3239.

Thanks a million.

[geyslan]

#3239 updated with findings.