Possible Data Race

[ZuhairORZaki]

Overview

In file: manager.go, there is a possible case of data race. In line 341, a goroutine with function manager.updateMachineInfo is started which updates shared resource machineInfo.

    // File: manager.go, Line: 341
    go m.updateMachineInfo(quitUpdateMachineInfo)
    // File: manager.go, Line: 383
    m.machineMu.Lock()
    m.machineInfo = *info
    m.machineMu.Unlock()

Also, in line 337, function manager.globalHousekeeping is invoked in another goroutine. It starts a call chain which can lead to calling function manager.resctrlManager.GetCollector where len(m.machineInfo.Topology) is passed as a parameter.

    // File: manager.go, Line: 337
    go m.globalHousekeeping(quitGlobalHousekeeping)
    // File: manager.go, Line: 408
    err := m.detectSubcontainers("/")
    // File: manager.go, Line: 1114
    err = m.createContainer(cont.Name, watcher.Raw)
    // File: manager.go, Line: 910
    return m.createContainerLocked(containerName, watchSource)
    // File: manager.go, Line: 958
    cont.resctrlCollector, err = m.resctrlManager.GetCollector(containerName, func() ([]string, error) {
		return cont.getContainerPids(m.inHostNamespace)
	}, len(m.machineInfo.Topology))

So, two separate goroutines are accessing machineInfo. But whereas the first goroutine safely updates machineInfo with proper locking, the second goroutine can access it without any kind of synchronization. This can cause unintended sequence of events.

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed - to improve global software supply chain security.

The bug is found by running the iCR tool by OpenRefactory, Inc. and then manually triaging the results.

[iwankgb]

@ZuhairORZaki, thank you for your report 🙇 It seems to be perfectly valid and I filed a PR to fix it.