Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 19 additions & 3 deletions pkg/security/resolvers/sbom/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ const (
)

// Data use the keep the result of a scan of a same workload across multiple
// container
// container. A *Data is shared across SBOMs that run the same image (via the
// dataCache), so it needs its own lock rather than relying on each SBOM's lock.
type Data struct {
mu sync.RWMutex
files fileQuerier
packages []sbomtypes.Package // per-package metadata (without the plain-text installed-file lists) kept for forwarding
}
Expand Down Expand Up @@ -435,8 +437,12 @@ func (r *Resolver) triggerForwarding(sbom *SBOM) {

// Snapshot the package metadata: the forwarded report outlives the
// lock and the backing slice keeps being mutated (LastAccess) at runtime.
// Take the Data lock because *Data is shared across SBOMs via the data
// cache, so the SBOM lock alone does not protect the packages slice.
sbom.data.mu.RLock()
packages := make([]sbomtypes.Package, len(sbom.data.packages))
copy(packages, sbom.data.packages)
sbom.data.mu.RUnlock()

// Create SBOM report and notify listeners
packagesReport := NewPackagesReport(packages, sbom.ContainerID)
Expand Down Expand Up @@ -776,6 +782,7 @@ func (r *Resolver) ResolvePackage(pc *model.ProcessContext, file *model.FileEven
if pkg != nil {
seclog.Tracef("file '%s' found in sbom for container '%s'", file.PathnameStr, sbom.ContainerID)

sbom.data.mu.Lock()
oldLastAccess := pkg.LastAccess
oldSuidBit := pkg.SuidBit
oldAccessedByRoot := pkg.AccessedByRoot
Expand All @@ -787,10 +794,17 @@ func (r *Resolver) ResolvePackage(pc *model.ProcessContext, file *model.FileEven
pkg.LastAccess = time.Now()
pkg.SuidBit = pkg.SuidBit || fs.FileMode(file.Mode)&04000 != 0
pkg.AccessedByRoot = pkg.AccessedByRoot || pc.UID == 0
// Snapshot the updated values before unlocking so the invalidation
// comparison below doesn't race with concurrent writers on the shared
// *Data (another container resolving a file from the same package).
newLastAccess := pkg.LastAccess
newSuidBit := pkg.SuidBit
newAccessedByRoot := pkg.AccessedByRoot
sbom.data.mu.Unlock()
Comment thread
pgimalac marked this conversation as resolved.

// Trigger forwarding debouncer to send updated SBOM to remote collector
if pkg.LastAccess.Sub(oldLastAccess) > r.cfg.SBOMResolverEnrichmentInterval ||
pkg.SuidBit != oldSuidBit || pkg.AccessedByRoot != oldAccessedByRoot {
if newLastAccess.Sub(oldLastAccess) > r.cfg.SBOMResolverEnrichmentInterval ||
newSuidBit != oldSuidBit || newAccessedByRoot != oldAccessedByRoot {
sbom.invalidated = true
}
}
Expand Down Expand Up @@ -855,6 +869,8 @@ func (r *Resolver) processPendingFileEvents(sbom *SBOM) {
seclog.Debugf("processing %d pending file events for container '%s'", len(events), sbom.ContainerID)

now := time.Now()
sbom.data.mu.Lock()
defer sbom.data.mu.Unlock()
for filePath, event := range events {
pkg := sbom.data.files.queryFile(filePath)
if pkg == nil {
Expand Down
54 changes: 54 additions & 0 deletions pkg/security/resolvers/sbom/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package sbom

import (
"fmt"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -322,3 +323,56 @@ func TestProcessPendingFileEventsEnrichesPackages(t *testing.T) {
t.Errorf("sbom was not marked for forwarding")
}
}

// TestSharedDataConcurrentForwardingAndResolve checks that the forwarding snapshot
// (copy of packages) and the package enrichment (LastAccess/SuidBit/AccessedByRoot
// writes) are safe when they run on different SBOMs sharing the same *Data via the
// dataCache. The SBOM lock is per-container, so without the Data-level lock the
// copy and the writes race on the shared packages slice.
func TestSharedDataConcurrentForwardingAndResolve(t *testing.T) {
data := newData([]sbomtypes.PackageWithInstalledFiles{{
Package: sbomtypes.Package{Name: "shadow-utils"},
InstalledFiles: []string{"/usr/bin/su"},
}}, false)

sbomA := NewSBOM("container-a", nil, "image:tag")
sbomA.data = data
sbomA.state.Store(computedState)

sbomB := NewSBOM("container-b", nil, "image:tag")
sbomB.data = data
sbomB.state.Store(computedState)

r := newPendingFileEventsResolver(t)

var wg sync.WaitGroup

// Writer: simulate ResolvePackage enriching packages on sbomA (writes
// LastAccess/SuidBit/AccessedByRoot on the shared Data).
wg.Go(func() {
for range 2000 {
r.queuePendingFileEvent("container-a", "/usr/bin/su", 04755, 0)
sbomA.Lock()
r.processPendingFileEvents(sbomA)
sbomA.Unlock()
}
})

// Reader: simulate triggerForwarding.func1 snapshotting packages on sbomB
// (reads the shared Data via copy()).
wg.Go(func() {
for range 2000 {
sbomB.Lock()
if sbomB.data != nil && len(sbomB.data.packages) > 0 {
sbomB.data.mu.RLock()
packages := make([]sbomtypes.Package, len(sbomB.data.packages))
copy(packages, sbomB.data.packages)
sbomB.data.mu.RUnlock()
_ = packages
}
sbomB.Unlock()
}
})

wg.Wait()
}
Loading