fix(security): protect shared SBOM Data with per-Data lock

Data races found via a race-detector-enabled build in staging (see #54333).

The *Data object is shared across SBOMs that run the same image via the
dataCache, but each SBOM has its own sync.RWMutex. So sbomA.Lock() and
sbomB.Lock() don't protect the shared Data.packages slice. The forwarding
debouncer's copy(packages, sbom.data.packages) races with ResolvePackage's
pkg.LastAccess = time.Now() when two containers share the same image.

Fix: add a sync.RWMutex to Data. Take data.mu.RLock() in the forwarding
snapshot (reader) and data.mu.Lock() in ResolvePackage and
processPendingFileEvents (writers). This mirrors the existing pattern in
fixedSizeQueue, which already has its own lock for the same reason.

Author: pgimalac

pkg/security/resolvers/sbom/resolver.go

@@ -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
 }
@@ -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)
@@ -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
@@ -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()
 
 		// 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
 		}
 	}
@@ -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 {

pkg/security/resolvers/sbom/resolver_test.go

@@ -9,6 +9,7 @@ package sbom
 
 import (
 	"fmt"
+	"sync"
 	"testing"
 	"time"
 
@@ -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()
+}