From 0f71f29673a0e6f65870053acb7d76420e624cfd Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Tue, 17 Oct 2023 10:04:30 +0800 Subject: [PATCH] Fix data race in FQDN ruleSyncTracker (#5583) ruleSyncTracker.Run() can update ruleSyncTracker.dirtyRules in-place, while ruleSyncTracker.getDirtyRules() returns the pointer of the set which could be read by other goroutines and leads to a data race like below: WARNING: DATA RACE Write at 0x00c000dd8180 by goroutine 276: runtime.mapdelete_faststr() /usr/local/go/src/runtime/map_faststr.go:301 +0x0 k8s.io/apimachinery/pkg/util/sets.Set[go.shape.string].Delete() /root/go/pkg/mod/k8s.io/apimachinery@v0.26.4/pkg/util/sets/set.go:62 +0x2ae antrea.io/antrea/pkg/agent/controller/networkpolicy.(*ruleSyncTracker).Run() /root/antrea/pkg/agent/controller/networkpolicy/fqdn.go:570 +0x1d2 antrea.io/antrea/pkg/agent/controller/networkpolicy.(*fqdnController).runRuleSyncTracker() /root/antrea/pkg/agent/controller/networkpolicy/fqdn.go:584 +0x4a antrea.io/antrea/pkg/agent/controller/networkpolicy.TestSyncDirtyRules.func1.4() /root/antrea/pkg/agent/controller/networkpolicy/fqdn_test.go:495 +0x44 Previous read at 0x00c000dd8180 by goroutine 271: reflect.maplen() /usr/local/go/src/runtime/map.go:1411 +0x0 reflect.Value.lenNonSlice() /usr/local/go/src/reflect/value.go:1720 +0x324 reflect.Value.Len() /usr/local/go/src/reflect/value.go:1709 +0x158f reflect.deepValueEqual() /usr/local/go/src/reflect/deepequal.go:139 +0x1571 reflect.DeepEqual() /usr/local/go/src/reflect/deepequal.go:237 +0x38b github.com/stretchr/testify/assert.ObjectsAreEqual() /root/go/pkg/mod/github.com/stretchr/testify@v1.8.4/assert/assertions.go:65 +0x172 github.com/stretchr/testify/assert.Equal() /root/go/pkg/mod/github.com/stretchr/testify@v1.8.4/assert/assertions.go:414 +0x1f7 antrea.io/antrea/pkg/agent/controller/networkpolicy.TestSyncDirtyRules.func1() /root/antrea/pkg/agent/controller/networkpolicy/fqdn_test.go:517 +0xb6f testing.tRunner() /usr/local/go/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /usr/local/go/src/testing/testing.go:1648 +0x44 Signed-off-by: Quan Tian --- pkg/agent/controller/networkpolicy/fqdn.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/agent/controller/networkpolicy/fqdn.go b/pkg/agent/controller/networkpolicy/fqdn.go index 7a534edc52c..dd270873f92 100644 --- a/pkg/agent/controller/networkpolicy/fqdn.go +++ b/pkg/agent/controller/networkpolicy/fqdn.go @@ -535,7 +535,8 @@ func (rst *ruleSyncTracker) subscribe(waitCh chan error, dirtyRules sets.String) func (rst *ruleSyncTracker) getDirtyRules() sets.String { rst.mutex.RLock() defer rst.mutex.RUnlock() - return rst.dirtyRules + // Must return a copy as the set can be updated in-place by Run func. + return rst.dirtyRules.Clone() } func (rst *ruleSyncTracker) Run(stopCh <-chan struct{}) {