Skip to content

Hongshi.guo/wina 1171 cpu info #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leika wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Hongshi.guo/wina 1171 cpu info #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e71f634
WINA-1171: fixes Windows CPU info collection
May 29, 2025
1b4d4c0
WINA-1171: release notes
May 30, 2025
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
19 changes: 19 additions & 0 deletions pkg/gohai/cpu/cpu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2014-present Datadog, Inc.

//go:build test
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reconsider the build constraint.

The //go:build test constraint restricts this test file to only compile when the test build tag is specified. This is unusual for test files, which typically compile during normal go test execution without special build tags.

Unless there's a specific reason for this constraint, remove it to allow normal test execution:

-//go:build test
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//go:build test
🤖 Prompt for AI Agents 
In pkg/gohai/cpu/cpu_test.go at line 5, the build constraint `//go:build test`
limits the test file to compile only with the `test` build tag, which is not
standard for Go test files. Remove this build constraint line entirely to allow
the test file to compile and run normally with the standard `go test` command.


package cpu

import (
"testing"

"github.com/DataDog/datadog-agent/pkg/gohai/utils"
"github.com/DataDog/datadog-agent/pkg/util/log"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -63,6 +66,22 @@ func TestCPUAsJSON(t *testing.T) {
var decodedCPU CPU
utils.RequireMarshallJSON(t, cpuInfo, &decodedCPU)

// output CPUCores, CPULogicalProcessors, Family, Mhz, Model, ModelName, Stepping, VendorID, CacheSizeKB, CacheSizeL1Bytes, CacheSizeL2Bytes, CacheSizeL3Bytes, CPUNumaNodes, CPUPkgs
log.Infof("CPUCores: %s", decodedCPU.CPUCores)
log.Infof("CPULogicalProcessors: %s", decodedCPU.CPULogicalProcessors)
log.Infof("Family: %s", decodedCPU.Family)
log.Infof("Mhz: %s", decodedCPU.Mhz)
log.Infof("Model: %s", decodedCPU.Model)
log.Infof("ModelName: %s", decodedCPU.ModelName)
log.Infof("Stepping: %s", decodedCPU.Stepping)
log.Infof("VendorID: %s", decodedCPU.VendorID)
log.Infof("CacheSizeKB: %s", decodedCPU.CacheSizeKB)
log.Infof("CacheSizeL1Bytes: %s", decodedCPU.CacheSizeL1Bytes)
log.Infof("CacheSizeL2Bytes: %s", decodedCPU.CacheSizeL2Bytes)
log.Infof("CacheSizeL3Bytes: %s", decodedCPU.CacheSizeL3Bytes)
log.Infof("CPUNumaNodes: %s", decodedCPU.CPUNumaNodes)
log.Infof("CPUPkgs: %s", decodedCPU.CPUPkgs)

utils.AssertDecodedValue(t, decodedCPU.CPUCores, &cpuInfo.CPUCores, "")
utils.AssertDecodedValue(t, decodedCPU.CPULogicalProcessors, &cpuInfo.CPULogicalProcessors, "")
utils.AssertDecodedValue(t, decodedCPU.Family, &cpuInfo.Family, "")
Expand Down
93 changes: 93 additions & 0 deletions pkg/gohai/cpu/cpu_windows.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// This file is licensed under the MIT License.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2025-present Datadog, Inc.

#include "cpu_windows.h"
#include <windows.h>
#include <stdio.h>

// computeCoresAndProcessors gets CPU information using Windows API
int computeCoresAndProcessors(CPU_INFO* cpuInfo) {
DWORD buflen = 0;
SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX* buffer = NULL;
BOOL done = FALSE;
DWORD returnLength = 0;
DWORD byteOffset = 0;

// Initialize the struct
memset(cpuInfo, 0, sizeof(CPU_INFO));

// First call to get required buffer size
GetLogicalProcessorInformationEx(RelationAll, NULL, &buflen);
if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
return GetLastError();
}

buffer = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX*)malloc(buflen);
Comment on lines +21 to +26
Copy link
Copy Markdown

@corgea corgea Bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐕 Corgea found a CWE-190: Integer Overflow or Wraparound vulnerability that is rated as 🔴 High.
View in Corgea.

🎟️Issue Explanation: The product performs a calculation that can produce an integer overflow or wraparound, when the logic assumes that the resulting value will always be larger than the original value. This can introduce other weaknesses when the calculation is used for resource management or execution control.

More issue detailsThe code risks integer overflow by assuming a calculated buffer size will stay positive, which could lead to resource mismanagement or improper execution control.

-"buflen" could exceed its maximum value during calculation, converting it into a negative.
-Overflow can trick "malloc(buflen)" into allocating insufficient memory.
-If exploited, this can disrupt tasks or expose sensitive data like processor info.
🪄Fix Suggestion:
Fix DetailsThe fix prevents integer overflow and zero-size allocations by checking if "buflen" is within valid limits before allocating memory. This ensures safe resource management and guards against potential overflow and wraparound vulnerabilities.
- An overflow check is added: "if (buflen == 0 || buflen > SIZE_MAX)" to ensure safe memory allocation.
- Returns "ERROR_INVALID_PARAMETER" if conditions are met, preventing further execution with invalid buffer sizes.
- Typecasting "buflen" to "(size_t)" during allocation ensures correct memory size handling.

Suggested change
GetLogicalProcessorInformationEx(RelationAll, NULL, &buflen);
if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
return GetLastError();
}
buffer = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX*)malloc(buflen);
GetLogicalProcessorInformationEx(RelationAll, NULL, &buflen);
if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
return GetLastError();
}
// Prevent integer overflow and zero-size allocation

if (buffer == NULL) {
return ERROR_OUTOFMEMORY;
}
Comment on lines +26 to +29
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add missing memory allocation header.

The code uses malloc but doesn't include <stdlib.h>. This could cause compilation warnings or errors.

 #include "cpu_windows.h"
 #include <windows.h>
 #include <stdio.h>
+#include <stdlib.h>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
buffer = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX*)malloc(buflen);
if (buffer == NULL) {
return ERROR_OUTOFMEMORY;
}
#include "cpu_windows.h"
#include <windows.h>
#include <stdio.h>
#include <stdlib.h>
🤖 Prompt for AI Agents 
In pkg/gohai/cpu/cpu_windows.c around lines 26 to 29, the code uses malloc for
memory allocation but does not include the <stdlib.h> header, which is required
for malloc declaration. Add #include <stdlib.h> at the top of the file to ensure
proper declaration and avoid compilation warnings or errors.


if (!GetLogicalProcessorInformationEx(RelationAll, buffer, &buflen)) {
free(buffer);
return GetLastError();
}

// Walk through the buffer
byteOffset = 0;
while (byteOffset < buflen) {
SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX* ptr = (SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX*)((char*)buffer + byteOffset);

switch (ptr->Relationship) {
case RelationProcessorCore:
cpuInfo->corecount++;
// Count logical processors in this core
for (WORD i = 0; i < ptr->Processor.GroupCount; i++) {
GROUP_AFFINITY* groupAffinity = &ptr->Processor.GroupMask[i];
ULONG64 mask = groupAffinity->Mask;
while (mask) {
if (mask & 1) {
cpuInfo->logicalcount++;
}
mask >>= 1;
}
}
break;

case RelationNumaNode:
cpuInfo->numaNodeCount++;
break;

case RelationCache:
switch (ptr->Cache.Level) {
case 1:
cpuInfo->l1CacheSize += ptr->Cache.CacheSize;
break;
case 2:
cpuInfo->l2CacheSize += ptr->Cache.CacheSize;
break;
case 3:
cpuInfo->l3CacheSize += ptr->Cache.CacheSize;
break;
}
break;

case RelationProcessorPackage:
cpuInfo->pkgcount++;
break;

case RelationGroup:
cpuInfo->relationGroups = ptr->Group.MaximumGroupCount;
for (WORD i = 0; i < ptr->Group.ActiveGroupCount; i++) {
cpuInfo->maxProcsInGroups += ptr->Group.GroupInfo[i].MaximumProcessorCount;
cpuInfo->activeProcsInGroups += ptr->Group.GroupInfo[i].ActiveProcessorCount;
}
break;
Comment on lines +79 to +85
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potential issue with group information handling.

The logic assumes there's only one RelationGroup entry, but the loop overwrites relationGroups and continues accumulating maxProcsInGroups and activeProcsInGroups. This could lead to incorrect values if multiple group entries exist.

             case RelationGroup:
-                cpuInfo->relationGroups = ptr->Group.MaximumGroupCount;
+                cpuInfo->relationGroups += ptr->Group.MaximumGroupCount;
                 for (WORD i = 0; i < ptr->Group.ActiveGroupCount; i++) {
                     cpuInfo->maxProcsInGroups += ptr->Group.GroupInfo[i].MaximumProcessorCount;
                     cpuInfo->activeProcsInGroups += ptr->Group.GroupInfo[i].ActiveProcessorCount;
                 }
                 break;

Or consider whether relationGroups should store the total count or just the maximum group count from the last entry, based on the intended semantics.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case RelationGroup:
cpuInfo->relationGroups = ptr->Group.MaximumGroupCount;
for (WORD i = 0; i < ptr->Group.ActiveGroupCount; i++) {
cpuInfo->maxProcsInGroups += ptr->Group.GroupInfo[i].MaximumProcessorCount;
cpuInfo->activeProcsInGroups += ptr->Group.GroupInfo[i].ActiveProcessorCount;
}
break;
case RelationGroup:
cpuInfo->relationGroups += ptr->Group.MaximumGroupCount;
for (WORD i = 0; i < ptr->Group.ActiveGroupCount; i++) {
cpuInfo->maxProcsInGroups += ptr->Group.GroupInfo[i].MaximumProcessorCount;
cpuInfo->activeProcsInGroups += ptr->Group.GroupInfo[i].ActiveProcessorCount;
}
break;
🤖 Prompt for AI Agents 
In pkg/gohai/cpu/cpu_windows.c around lines 79 to 85, the code assumes a single
RelationGroup entry but may process multiple, causing relationGroups to be
overwritten and maxProcsInGroups and activeProcsInGroups to accumulate
incorrectly. Modify the code to either accumulate relationGroups across all
RelationGroup entries or clarify if it should only store the maximum group count
from the last entry, ensuring consistent and correct aggregation of group
information.

}

byteOffset += ptr->Size;
}

free(buffer);
return 0;
}
201 changes: 101 additions & 100 deletions pkg/gohai/cpu/cpu_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,38 @@
// This file is licensed under the MIT License.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright © 2015 Kentaro Kuribayashi <kentarok@gmail.com>
// Copyright 2014-present Datadog, Inc.

//go:build windows

package cpu

/*
#cgo LDFLAGS: -lkernel32
#include "cpu_windows.h"
*/
import "C"
import (
"fmt"
"regexp"
"strconv"
"strings"
"unsafe"

"golang.org/x/sys/windows"

"github.com/DataDog/datadog-agent/pkg/gohai/utils"
"github.com/DataDog/datadog-agent/pkg/util/log"
"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"
)

const registryHive = "HARDWARE\\DESCRIPTION\\System\\CentralProcessor\\0"

// see https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getlogicalprocessorinformationex
const (
// RelationProcessorCore retrieves information about logical processors
// that share a single processor core.
RelationProcessorCore = 0
// RelationNumaNode retrieves information about logical processors
// that are part of the same NUMA node.
RelationNumaNode = 1
// RelationCache retrieves information about logical processors
// that share a cache.
RelationCache = 2
// RelationProcessorPackage retrieves information about logical processors
// that share a physical package.
RelationProcessorPackage = 3
// RelationGroup retrieves information about logical processors
// that share a processor group.
RelationGroup = 4
)

// SYSTEM_INFO contains information about the current computer system.
// systemInfo contains information about the current computer system.
// This includes the architecture and type of the processor, the number
// of processors in the system, the page size, and other such information.
// see https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-system_info
//
//nolint:revive
type SYSTEM_INFO struct {
type systemInfo struct {
wProcessorArchitecture uint16
wReserved uint16
dwPageSize uint32
Expand All @@ -59,96 +46,110 @@ type SYSTEM_INFO struct {
wProcessorRevision uint16
}

// CPU_INFO contains information about cpu, eg. number of cores, cache size
//
//nolint:revive
type CPU_INFO struct {
numaNodeCount int // number of NUMA nodes
pkgcount int // number of packages (physical CPUS)
corecount int // total number of cores
logicalcount int // number of logical CPUS
l1CacheSize uint32 // layer 1 cache size
l2CacheSize uint32 // layer 2 cache size
l3CacheSize uint32 // layer 3 cache size
//nolint:unused
relationGroups int // number of cpu relation groups
//nolint:unused
maxProcsInGroups int // max number of processors
//nolint:unused
activeProcsInGroups int // active processors
// cpuInfo holds CPU information
type cpuInfo struct {
corecount int
logicalcount int
pkgcount int
numaNodeCount int
relationGroups int
maxProcsInGroups int
activeProcsInGroups int
l1CacheSize uint64
l2CacheSize uint64
l3CacheSize uint64
VendorID string
ModelName string
Mhz float64
Family string
Model string
Stepping string
}

func countBits(num uint64) (count int) {
count = 0
for num > 0 {
if (num & 0x1) == 1 {
count++
}
num >>= 1
func extract(caption, field string) string {
re := regexp.MustCompile(fmt.Sprintf("%s [0-9]* ", field))
matches := re.FindStringSubmatch(caption)
if len(matches) > 0 {
return strings.Split(matches[0], " ")[1]
}
return
}

func getSystemInfo() (si SYSTEM_INFO) {
var mod = windows.NewLazyDLL("kernel32.dll")
var gsi = mod.NewProc("GetSystemInfo")

// syscall does not fail
//nolint:errcheck
gsi.Call(uintptr(unsafe.Pointer(&si)))
return
return ""
}

func getCPUInfo() *Info {
cpuInfo := &Info{
CacheSizeKB: utils.NewErrorValue[uint64](utils.ErrNotCollectable),
var cInfo cpuInfo
var cCpuInfo C.CPU_INFO

ret := C.computeCoresAndProcessors(&cCpuInfo)
if ret != 0 {
log.Errorf("failed to get CPU information, error code: %d", ret)
return &Info{}
}

// Copy C struct values to Go struct
cInfo.corecount = int(cCpuInfo.corecount)
cInfo.logicalcount = int(cCpuInfo.logicalcount)
cInfo.pkgcount = int(cCpuInfo.pkgcount)
cInfo.numaNodeCount = int(cCpuInfo.numaNodeCount)
cInfo.relationGroups = int(cCpuInfo.relationGroups)
cInfo.maxProcsInGroups = int(cCpuInfo.maxProcsInGroups)
cInfo.activeProcsInGroups = int(cCpuInfo.activeProcsInGroups)
cInfo.l1CacheSize = uint64(cCpuInfo.l1CacheSize)
cInfo.l2CacheSize = uint64(cCpuInfo.l2CacheSize)
cInfo.l3CacheSize = uint64(cCpuInfo.l3CacheSize)

// Get additional CPU information from Windows registry
k, err := registry.OpenKey(registry.LOCAL_MACHINE,
registryHive,
registry.QUERY_VALUE)
if err == nil {
defer k.Close()

dw, _, err := k.GetIntegerValue("~MHz")
cpuInfo.Mhz = utils.NewValueFrom(float64(dw), err)

s, _, err := k.GetStringValue("ProcessorNameString")
cpuInfo.ModelName = utils.NewValueFrom(s, err)

s, _, err = k.GetStringValue("VendorIdentifier")
cpuInfo.VendorID = utils.NewValueFrom(s, err)

s, _, err = k.GetStringValue("Identifier")
if err == nil {
cpuInfo.Family = utils.NewValue(extract(s, "Family"))
} else {
cpuInfo.Family = utils.NewErrorValue[string](err)
}
} else {
cpuInfo.Mhz = utils.NewErrorValue[float64](err)
cpuInfo.ModelName = utils.NewErrorValue[string](err)
cpuInfo.VendorID = utils.NewErrorValue[string](err)
cpuInfo.Family = utils.NewErrorValue[string](err)
if err != nil {
log.Errorf("failed to open registry key: %v", err)
return &Info{}
}
Comment on lines +100 to 107
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Preserve CPU data from C function when registry access fails.

The current implementation returns an empty Info{} struct when the registry key cannot be opened, discarding potentially valid CPU topology data already retrieved from the C function (cores, logical processors, cache sizes, etc.).

Consider continuing with the data collection even if the registry access fails:

 	// Get additional CPU information from Windows registry
 	k, err := registry.OpenKey(registry.LOCAL_MACHINE,
 		registryHive,
 		registry.QUERY_VALUE)
 	if err != nil {
 		log.Errorf("failed to open registry key: %v", err)
-		return &Info{}
+		// Continue with partial data from C function
+	} else {
+		defer k.Close()
+		
+		if mhz, _, err := k.GetIntegerValue("~MHz"); err == nil {
+			cInfo.Mhz = float64(mhz)
+		}
+		// ... rest of registry reads ...
 	}
-	defer k.Close()

This ensures that CPU topology information is still available even if registry-based details (MHz, vendor ID, etc.) cannot be retrieved.

🤖 Prompt for AI Agents 
In pkg/gohai/cpu/cpu_windows.go around lines 100 to 107, the code returns an
empty Info struct when failing to open the registry key, discarding CPU data
obtained from the C function. Modify the error handling to log the error but
continue execution, returning the partially populated Info struct with the data
collected so far instead of an empty one. This preserves CPU topology
information even if registry access fails.

defer k.Close()

cpus, cpuerr := computeCoresAndProcessors()
cpuInfo.CPUPkgs = utils.NewValueFrom(uint64(cpus.pkgcount), cpuerr)
cpuInfo.CPUNumaNodes = utils.NewValueFrom(uint64(cpus.numaNodeCount), cpuerr)
cpuInfo.CPUCores = utils.NewValueFrom(uint64(cpus.corecount), cpuerr)
cpuInfo.CPULogicalProcessors = utils.NewValueFrom(uint64(cpus.logicalcount), cpuerr)
cpuInfo.CacheSizeL1Bytes = utils.NewValueFrom(uint64(cpus.l1CacheSize), cpuerr)
cpuInfo.CacheSizeL2Bytes = utils.NewValueFrom(uint64(cpus.l2CacheSize), cpuerr)
cpuInfo.CacheSizeL3Bytes = utils.NewValueFrom(uint64(cpus.l3CacheSize), cpuerr)
if mhz, _, err := k.GetIntegerValue("~MHz"); err == nil {
cInfo.Mhz = float64(mhz)
}

si := getSystemInfo()
cpuInfo.Model = utils.NewValue(strconv.Itoa(int((si.wProcessorRevision >> 8) & 0xFF)))
cpuInfo.Stepping = utils.NewValue(strconv.Itoa(int(si.wProcessorRevision & 0xFF)))
if name, _, err := k.GetStringValue("ProcessorNameString"); err == nil {
cInfo.ModelName = name
}

return cpuInfo
}
if vendor, _, err := k.GetStringValue("VendorIdentifier"); err == nil {
cInfo.VendorID = vendor
}

func extract(caption, field string) string {
re := regexp.MustCompile(fmt.Sprintf("%s [0-9]* ", field))
return strings.Split(re.FindStringSubmatch(caption)[0], " ")[1]
if identifier, _, err := k.GetStringValue("Identifier"); err == nil {
cInfo.Family = extract(identifier, "Family")
}

// Get system info for model and stepping
var si systemInfo
var mod = windows.NewLazyDLL("kernel32.dll")
var gsi = mod.NewProc("GetSystemInfo")
//nolint:errcheck
gsi.Call(uintptr(unsafe.Pointer(&si)))

cInfo.Model = strconv.Itoa(int((si.wProcessorRevision >> 8) & 0xFF))
cInfo.Stepping = strconv.Itoa(int(si.wProcessorRevision & 0xFF))

// Convert to Info struct
info := &Info{
VendorID: utils.NewValue(cInfo.VendorID),
ModelName: utils.NewValue(cInfo.ModelName),
CPUCores: utils.NewValue(uint64(cInfo.corecount)),
CPULogicalProcessors: utils.NewValue(uint64(cInfo.logicalcount)),
Mhz: utils.NewValue(cInfo.Mhz),
Family: utils.NewValue(cInfo.Family),
Model: utils.NewValue(cInfo.Model),
Stepping: utils.NewValue(cInfo.Stepping),
CPUPkgs: utils.NewValue(uint64(cInfo.pkgcount)),
CPUNumaNodes: utils.NewValue(uint64(cInfo.numaNodeCount)),
CacheSizeL1Bytes: utils.NewValue(cInfo.l1CacheSize),
CacheSizeL2Bytes: utils.NewValue(cInfo.l2CacheSize),
CacheSizeL3Bytes: utils.NewValue(cInfo.l3CacheSize),
CacheSizeKB: utils.NewErrorValue[uint64](utils.ErrNotCollectable),
}

return info
}
Loading