Skip to content

Commit 1bf228d

Browse files
authored
fix: lock fixedSizeQueue to prevent race on shared SBOM negative cache (#54646)
### What does this PR do? Adds an internal mutex to `fixedSizeQueue` (`pkg/security/resolvers/sbom/file_querier.go`), used as `fileQuerier.lastNegativeCache`. This queue is cached and shared across containers running the same image, so it needs its own lock instead of relying on each container's separate `SBOM` lock. ### Motivation Fix a race, found via a race-detector-enabled build in staging. ### Describe how you validated your changes Added `TestFixedSizeQueueConcurrentAccess`; fails under `-race` before this fix, passes after. Co-authored-by: pierre.gimalac <pierre.gimalac@datadoghq.com>
1 parent 62f913c commit 1bf228d

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

pkg/security/resolvers/sbom/file_querier.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package sbom
1111
import (
1212
"slices"
1313
"strings"
14+
"sync"
1415

1516
sbomtypes "github.com/DataDog/datadog-agent/pkg/security/resolvers/sbom/types"
1617
"github.com/DataDog/datadog-agent/pkg/security/seclog"
@@ -138,7 +139,10 @@ func (fq *fileQuerier) len() int {
138139
return len(fq.files)
139140
}
140141

142+
// fixedSizeQueue is shared across containers with the same image (see newData),
143+
// so it needs its own lock rather than relying on the caller's SBOM lock.
141144
type fixedSizeQueue[T comparable] struct {
145+
mu sync.Mutex
142146
queue []T
143147
maxSize int
144148
}
@@ -148,6 +152,9 @@ func newFixedSizeQueue[T comparable](maxSize int) *fixedSizeQueue[T] {
148152
}
149153

150154
func (q *fixedSizeQueue[T]) push(value T) {
155+
q.mu.Lock()
156+
defer q.mu.Unlock()
157+
151158
if len(q.queue) == q.maxSize {
152159
q.queue = q.queue[1:]
153160
}
@@ -160,5 +167,8 @@ func (q *fixedSizeQueue[T]) contains(value T) bool {
160167
return false
161168
}
162169

170+
q.mu.Lock()
171+
defer q.mu.Unlock()
172+
163173
return slices.Contains(q.queue, value)
164174
}

pkg/security/resolvers/sbom/file_querier_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package sbom
99

1010
import (
11+
"sync"
1112
"testing"
1213

1314
sbomtypes "github.com/DataDog/datadog-agent/pkg/security/resolvers/sbom/types"
@@ -59,3 +60,40 @@ func TestQueryFileUsrMerge(t *testing.T) {
5960
t.Errorf("queryFile(/usr/bin/mount) attributed to %q on a non-usr-merged layout, want no match", pkg.Name)
6061
}
6162
}
63+
64+
// TestFixedSizeQueueConcurrentAccess calls push/contains concurrently on a
65+
// shared fixedSizeQueue, as happens when containers share an image. Run with -race.
66+
func TestFixedSizeQueueConcurrentAccess(t *testing.T) {
67+
q := newFixedSizeQueue[uint64](2)
68+
69+
const goroutines = 50
70+
const iterations = 200
71+
72+
var wg sync.WaitGroup
73+
74+
for g := 0; g < goroutines; g++ {
75+
seed := uint64(g)
76+
wg.Go(func() {
77+
for i := uint64(0); i < iterations; i++ {
78+
q.push(seed*iterations + i)
79+
}
80+
})
81+
}
82+
83+
for g := 0; g < goroutines; g++ {
84+
seed := uint64(g)
85+
wg.Go(func() {
86+
for i := uint64(0); i < iterations; i++ {
87+
q.contains(seed*iterations + i)
88+
}
89+
})
90+
}
91+
92+
wg.Wait()
93+
94+
// The queue must never grow past its configured bound, even under
95+
// concurrent access.
96+
if got := len(q.queue); got > 2 {
97+
t.Errorf("queue length = %d, want at most 2", got)
98+
}
99+
}

0 commit comments

Comments
 (0)