From d1ef25255d9acc08c8d5e2d57e9d81a8b9a64431 Mon Sep 17 00:00:00 2001 From: "Dementiev, Roman" Date: Thu, 11 Dec 2025 11:03:45 +0100 Subject: [PATCH 01/19] support 64-bit bar Change-Id: Id95df391f74b819b8ae379211b543cc1ab024b01 --- src/pci.h | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/pci.h b/src/pci.h index b665b597..e2ff2321 100644 --- a/src/pci.h +++ b/src/pci.h @@ -234,13 +234,42 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) DBG(2, "offset 0x" , std::hex , offset , " cap_id: 0x" , header.fields.cap_id , " vsec_id: 0x", header.fields.vsec_id, " entryID: 0x" , std::hex , header.fields.entryID , std::dec); if (matchFunc(header)) { - DBG(2, ".... found match"); + DBG(2, ".... found match. tBIR = ", header.fields.tBIR); auto barOffset = 0x10 + header.fields.tBIR * 4; uint32 bar = 0; if (h.read32(barOffset, &bar) == sizeof(uint32) && bar != 0) // read bar { - bar &= ~4095; - processFunc(bar, header); + DBG(2, "bar = 0x", std::hex, bar, std::dec); + if (extract_bits_32(bar, 0, 0) == 0) // memory space + { + auto type = extract_bits_32(bar, 2, 1); + if (type == 0) // 32-bit address + { + bar &= ~0xfull; + processFunc(bar, header); + } + else if (type == 2) // 64-bit address + { + uint32 bar_high = 0; + if (h.read32(barOffset + 4, &bar_high) == sizeof(uint32)) + { + uint64 full_bar = uint64(bar_high); + full_bar <<= 32; + full_bar |= uint64(bar); + full_bar &= ~0xfull; + DBG(2, " full_bar = 0x", std::hex, full_bar, std::dec); + processFunc(full_bar, header); + } + else + { + std::cerr << "Error: can't read high part of 64-bit bar from offset " << (barOffset + 4) << " \n"; + } + } + else + { + std::cerr << "Error: unknown bar type " << type << " \n"; + } + } } else { From d1df076b6b80d002adb17e29fa05722cbc94815a Mon Sep 17 00:00:00 2001 From: Roman Dementiev Date: Thu, 11 Dec 2025 11:11:51 +0100 Subject: [PATCH 02/19] Update src/pci.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/pci.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pci.h b/src/pci.h index e2ff2321..af21217c 100644 --- a/src/pci.h +++ b/src/pci.h @@ -253,9 +253,7 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) uint32 bar_high = 0; if (h.read32(barOffset + 4, &bar_high) == sizeof(uint32)) { - uint64 full_bar = uint64(bar_high); - full_bar <<= 32; - full_bar |= uint64(bar); + uint64 full_bar = (uint64(bar_high) << 32) | uint64(bar); full_bar &= ~0xfull; DBG(2, " full_bar = 0x", std::hex, full_bar, std::dec); processFunc(full_bar, header); From 2a4176494645584bb41195e5187ef547847228c6 Mon Sep 17 00:00:00 2001 From: Roman Dementiev Date: Thu, 11 Dec 2025 11:12:15 +0100 Subject: [PATCH 03/19] Update src/pci.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/pci.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pci.h b/src/pci.h index af21217c..7a4c44dc 100644 --- a/src/pci.h +++ b/src/pci.h @@ -265,7 +265,7 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) } else { - std::cerr << "Error: unknown bar type " << type << " \n"; + std::cerr << "Error: unknown bar type " << type << " at bar offset " << barOffset << " \n"; } } } From fbaddac9e77e434b61bd1ef30a489e0edde3ecf4 Mon Sep 17 00:00:00 2001 From: "Dementiev, Roman" Date: Thu, 11 Dec 2025 11:15:08 +0100 Subject: [PATCH 04/19] print offsets in hex Change-Id: Id6c3901fa6488247cecad955ac83ec70ccfe7a6b --- src/pci.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pci.h b/src/pci.h index 7a4c44dc..a5ceaa46 100644 --- a/src/pci.h +++ b/src/pci.h @@ -260,18 +260,18 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) } else { - std::cerr << "Error: can't read high part of 64-bit bar from offset " << (barOffset + 4) << " \n"; + std::cerr << "Error: can't read high part of 64-bit bar from offset 0x" << std::hex << (barOffset + 4) << std::dec << " \n"; } } else { - std::cerr << "Error: unknown bar type " << type << " at bar offset " << barOffset << " \n"; + std::cerr << "Error: unknown bar type " << type << " at bar offset 0x" << std::hex << barOffset << std::dec << " \n"; } } } else { - std::cerr << "Error: can't read bar from offset " << barOffset << " \n"; + std::cerr << "Error: can't read bar from offset 0x" << std::hex << barOffset << std::dec << " \n"; } } const uint64 lastOffset = offset; From 1ebd39872ae18ad2e7d0227a41422b9ec65f7a1e Mon Sep 17 00:00:00 2001 From: Roman Dementiev Date: Thu, 11 Dec 2025 11:28:36 +0100 Subject: [PATCH 05/19] Update src/pci.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/pci.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pci.h b/src/pci.h index a5ceaa46..a998454c 100644 --- a/src/pci.h +++ b/src/pci.h @@ -265,6 +265,8 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) } else { + // BAR type 1 is reserved by the PCI specification and is not valid. + // Any unknown BAR type (including type 1) is treated as an error. std::cerr << "Error: unknown bar type " << type << " at bar offset 0x" << std::hex << barOffset << std::dec << " \n"; } } From e6e1cda0a77dbb79cec076625a35189841d6656c Mon Sep 17 00:00:00 2001 From: "Dementiev, Roman" Date: Thu, 11 Dec 2025 16:01:15 +0100 Subject: [PATCH 06/19] extend UncorePMUDiscovery to support dies Change-Id: I2b2b8a80fc28aa4386b0ae892de8942fc8a9de16 --- src/cpucounters.cpp | 214 ++++++++++++++++++----------------- src/uncore_pmu_discovery.cpp | 34 +++--- src/uncore_pmu_discovery.h | 59 ++++++---- 3 files changed, 167 insertions(+), 140 deletions(-) diff --git a/src/cpucounters.cpp b/src/cpucounters.cpp index f5817da5..67c756d4 100644 --- a/src/cpucounters.cpp +++ b/src/cpucounters.cpp @@ -2307,27 +2307,30 @@ void PCM::initUncorePMUsDirect() { if (uncorePMUDiscovery.get()) { - for (size_t box = 0; box < uncorePMUDiscovery->getNumBoxes(pmuType, s); ++box) + for (size_t die = 0; die < uncorePMUDiscovery->getNumDies(s); ++die) { - if (uncorePMUDiscovery->getBoxAccessType(pmuType, s, box) == UncorePMUDiscovery::accessTypeEnum::MSR - && uncorePMUDiscovery->getBoxNumRegs(pmuType, s, box) >= 4) + for (size_t box = 0; box < uncorePMUDiscovery->getNumBoxes(pmuType, s, die); ++box) { - out.push_back( - std::make_shared( - std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box, 0)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box, 1)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box, 2)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box, 3)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, box, 0)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, box, 1)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, box, 2)), - std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, box, 3)), - std::shared_ptr(), - std::shared_ptr(), - (filter0 < 0) ? std::shared_ptr() : std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, box) + filter0) // filters not supported by discovery - ) - ); + if (uncorePMUDiscovery->getBoxAccessType(pmuType, s, die, box) == UncorePMUDiscovery::accessTypeEnum::MSR + && uncorePMUDiscovery->getBoxNumRegs(pmuType, s, die, box) >= 4) + { + out.push_back( + std::make_shared( + std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box, 0)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box, 1)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box, 2)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box, 3)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, die, box, 0)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, die, box, 1)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, die, box, 2)), + std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(pmuType, s, die, box, 3)), + std::shared_ptr(), + std::shared_ptr(), + (filter0 < 0) ? std::shared_ptr() : std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(pmuType, s, die, box) + filter0) // filters not supported by discovery + ) + ); + } } } } @@ -2918,17 +2921,17 @@ void PCM::initUncorePMUsDirect() { if (uncorePMUDiscovery.get()) { - auto createCXLPMU = [this](const uint32 s, const unsigned BoxType, const size_t pos) -> UncorePMU + auto createCXLPMU = [this](const uint32 s, const size_t die, const unsigned BoxType, const size_t pos) -> UncorePMU { std::vector > CounterControlRegs, CounterValueRegs; - const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, s, pos); - const auto unitControlAddr = uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, pos); + const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, s, die, pos); + const auto unitControlAddr = uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, die, pos); const auto unitControlAddrAligned = unitControlAddr & ~4095ULL; auto handle = std::make_shared(unitControlAddrAligned, CXL_PMON_SIZE, false); for (size_t r = 0; r < n_regs; ++r) { - CounterControlRegs.push_back(std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, pos, r) - unitControlAddrAligned)); - CounterValueRegs.push_back(std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(BoxType, s, pos, r) - unitControlAddrAligned)); + CounterControlRegs.push_back(std::make_shared(handle, uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, die, pos, r) - unitControlAddrAligned)); + CounterValueRegs.push_back(std::make_shared(handle, uncorePMUDiscovery->getBoxCtrAddr(BoxType, s, die, pos, r) - unitControlAddrAligned)); } return UncorePMU(std::make_shared(handle, unitControlAddr - unitControlAddrAligned), CounterControlRegs, CounterValueRegs); }; @@ -2940,18 +2943,19 @@ void PCM::initUncorePMUsDirect() case PCM::GNR: case PCM::GNR_D: case PCM::SRF: + for (size_t die = 0; die < uncorePMUDiscovery->getNumDies(s); ++die) { - const auto n_units = (std::min)(uncorePMUDiscovery->getNumBoxes(SPR_CXLCM_BOX_TYPE, s), - uncorePMUDiscovery->getNumBoxes(SPR_CXLDP_BOX_TYPE, s)); + const auto n_units = (std::min)(uncorePMUDiscovery->getNumBoxes(SPR_CXLCM_BOX_TYPE, s, die), + uncorePMUDiscovery->getNumBoxes(SPR_CXLDP_BOX_TYPE, s, die)); for (size_t pos = 0; pos < n_units; ++pos) { try { - cxlPMUs[s].push_back(std::make_pair(createCXLPMU(s, SPR_CXLCM_BOX_TYPE, pos), createCXLPMU(s, SPR_CXLDP_BOX_TYPE, pos))); + cxlPMUs[s].push_back(std::make_pair(createCXLPMU(s, die, SPR_CXLCM_BOX_TYPE, pos), createCXLPMU(s, die, SPR_CXLDP_BOX_TYPE, pos))); } catch (const std::exception& e) { - std::cerr << "CXL PMU initialization for socket " << s << " at position " << pos << " failed: " << e.what() << std::endl; + std::cerr << "CXL PMU initialization for socket " << s << " die " << die << " at position " << pos << " failed: " << e.what() << std::endl; } } } @@ -7645,53 +7649,56 @@ void PCM::getPCICFGPMUsFromDiscovery(const unsigned int BoxType, const size_t s, { if (uncorePMUDiscovery.get()) { - const auto numBoxes = uncorePMUDiscovery->getNumBoxes(BoxType, s); - for (size_t pos = 0; pos < numBoxes; ++pos) + for (size_t die = 0; die < uncorePMUDiscovery->getNumDies(s); ++die) { - if (uncorePMUDiscovery->getBoxAccessType(BoxType, s, pos) == UncorePMUDiscovery::accessTypeEnum::PCICFG) + const auto numBoxes = uncorePMUDiscovery->getNumBoxes(BoxType, s, die); + for (size_t pos = 0; pos < numBoxes; ++pos) { - std::vector > CounterControlRegs, CounterValueRegs; - const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, s, pos); - auto makeRegister = [&pos, &numBoxes, &BoxType, &s](const uint64 rawAddr) + if (uncorePMUDiscovery->getBoxAccessType(BoxType, s, die, pos) == UncorePMUDiscovery::accessTypeEnum::PCICFG) { -#ifndef PCI_ENABLE - constexpr auto PCI_ENABLE = 0x80000000ULL; -#endif - UncorePMUDiscovery::PCICFGAddress Addr; - Addr.raw = rawAddr; - if ((Addr.raw & PCI_ENABLE) == 0) + std::vector > CounterControlRegs, CounterValueRegs; + const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, s, die, pos); + auto makeRegister = [&pos, &numBoxes, &BoxType, &s](const uint64 rawAddr) { - std::cerr << "PCM Error: PCI_ENABLE bit not set in address 0x" << std::hex << Addr.raw << std::dec << "\n"; - std::cerr << "This is likely a bug in the uncore PMU discovery BIOS table. Contact your BIOS vendor.\n"; - std::cerr << "Socket: " << s << "\n"; - std::cerr << "Box type: " << BoxType << "\n"; - std::cerr << "Box position: " << pos << "/" << numBoxes << "\n"; - std::cerr << "Address: " << Addr.getStr() << "\n"; + #ifndef PCI_ENABLE + constexpr auto PCI_ENABLE = 0x80000000ULL; + #endif + UncorePMUDiscovery::PCICFGAddress Addr; + Addr.raw = rawAddr; + if ((Addr.raw & PCI_ENABLE) == 0) + { + std::cerr << "PCM Error: PCI_ENABLE bit not set in address 0x" << std::hex << Addr.raw << std::dec << "\n"; + std::cerr << "This is likely a bug in the uncore PMU discovery BIOS table. Contact your BIOS vendor.\n"; + std::cerr << "Socket: " << s << "\n"; + std::cerr << "Box type: " << BoxType << "\n"; + std::cerr << "Box position: " << pos << "/" << numBoxes << "\n"; + std::cerr << "Address: " << Addr.getStr() << "\n"; + return std::shared_ptr(); + } + try { + auto handle = std::make_shared(0, (uint32)Addr.fields.bus, + (uint32)Addr.fields.device, + (uint32)Addr.fields.function); + assert(handle.get()); + DBG(3, "opened bdf ", Addr.getStr()); + return std::make_shared(handle, (size_t)Addr.fields.offset); + } + catch (...) + { + DBG(3, "error opening bdf ", Addr.getStr()); + } return std::shared_ptr(); - } - try { - auto handle = std::make_shared(0, (uint32)Addr.fields.bus, - (uint32)Addr.fields.device, - (uint32)Addr.fields.function); - assert(handle.get()); - DBG(3, "opened bdf ", Addr.getStr()); - return std::make_shared(handle, (size_t)Addr.fields.offset); - } - catch (...) + }; + auto boxCtlRegister = makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, die, pos)); + if (boxCtlRegister.get()) { - DBG(3, "error opening bdf ", Addr.getStr()); - } - return std::shared_ptr(); - }; - auto boxCtlRegister = makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, pos)); - if (boxCtlRegister.get()) - { - for (size_t r = 0; r < n_regs; ++r) - { - CounterControlRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, pos, r))); - CounterValueRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtrAddr(BoxType, s, pos, r))); + for (size_t r = 0; r < n_regs; ++r) + { + CounterControlRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, s, die, pos, r))); + CounterValueRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtrAddr(BoxType, s, die, pos, r))); + } + f(UncorePMU(boxCtlRegister, CounterControlRegs, CounterValueRegs)); } - f(UncorePMU(boxCtlRegister, CounterControlRegs, CounterValueRegs)); } } } @@ -8276,48 +8283,51 @@ void ServerUncorePMUs::initDirect(uint32 socket_, const PCM * pcm) const auto BoxType = SPR_IMC_BOX_TYPE; if (uncorePMUDiscovery.get()) { - const auto numBoxes = uncorePMUDiscovery->getNumBoxes(BoxType, socket_); - for (size_t pos = 0; pos < numBoxes; ++pos) + for (size_t die = 0; die < uncorePMUDiscovery->getNumDies(socket_); ++die) { - if (uncorePMUDiscovery->getBoxAccessType(BoxType, socket_, pos) == UncorePMUDiscovery::accessTypeEnum::MMIO) + const auto numBoxes = uncorePMUDiscovery->getNumBoxes(BoxType, socket_, die); + for (size_t pos = 0; pos < numBoxes; ++pos) { - std::vector > CounterControlRegs, CounterValueRegs; - const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, socket_, pos); - auto makeRegister = [](const uint64 rawAddr, const uint32 bits) -> std::shared_ptr + if (uncorePMUDiscovery->getBoxAccessType(BoxType, socket_, die, pos) == UncorePMUDiscovery::accessTypeEnum::MMIO) { - const auto mapSize = SERVER_MC_CH_PMON_SIZE; - const auto alignedAddr = rawAddr & ~4095ULL; - const auto alignDelta = rawAddr & 4095ULL; - try { - auto handle = std::make_shared(alignedAddr, mapSize, false); - assert(handle.get()); - switch (bits) + std::vector > CounterControlRegs, CounterValueRegs; + const auto n_regs = uncorePMUDiscovery->getBoxNumRegs(BoxType, socket_, die, pos); + auto makeRegister = [](const uint64 rawAddr, const uint32 bits) -> std::shared_ptr + { + const auto mapSize = SERVER_MC_CH_PMON_SIZE; + const auto alignedAddr = rawAddr & ~4095ULL; + const auto alignDelta = rawAddr & 4095ULL; + try { + auto handle = std::make_shared(alignedAddr, mapSize, false); + assert(handle.get()); + switch (bits) + { + case 32: + return std::make_shared(handle, (size_t)alignDelta); + case 64: + return std::make_shared(handle, (size_t)alignDelta); + } + } + catch (...) { - case 32: - return std::make_shared(handle, (size_t)alignDelta); - case 64: - return std::make_shared(handle, (size_t)alignDelta); } - } - catch (...) - { - } - return std::shared_ptr(); - }; + return std::shared_ptr(); + }; - auto boxCtlRegister = makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, pos), 32); - if (boxCtlRegister.get()) - { - for (size_t r = 0; r < n_regs; ++r) + auto boxCtlRegister = makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, die, pos), 32); + if (boxCtlRegister.get()) { - CounterControlRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, pos, r), 32)); - CounterValueRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtrAddr(BoxType, socket_, pos, r), 64)); + for (size_t r = 0; r < n_regs; ++r) + { + CounterControlRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, die, pos, r), 32)); + CounterValueRegs.push_back(makeRegister(uncorePMUDiscovery->getBoxCtrAddr(BoxType, socket_, die, pos, r), 64)); + } + imcPMUs.push_back(UncorePMU(boxCtlRegister, + CounterControlRegs, + CounterValueRegs, + makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, die, pos) + SERVER_MC_CH_PMON_FIXED_CTL_OFFSET, 32), + makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, die, pos) + SERVER_MC_CH_PMON_FIXED_CTR_OFFSET, 64))); } - imcPMUs.push_back(UncorePMU(boxCtlRegister, - CounterControlRegs, - CounterValueRegs, - makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, pos) + SERVER_MC_CH_PMON_FIXED_CTL_OFFSET, 32), - makeRegister(uncorePMUDiscovery->getBoxCtlAddr(BoxType, socket_, pos) + SERVER_MC_CH_PMON_FIXED_CTR_OFFSET, 64))); } } } diff --git a/src/uncore_pmu_discovery.cpp b/src/uncore_pmu_discovery.cpp index fd968ef4..bf569070 100644 --- a/src/uncore_pmu_discovery.cpp +++ b/src/uncore_pmu_discovery.cpp @@ -16,8 +16,10 @@ UncorePMUDiscovery::UncorePMUDiscovery() return; } const auto debug = (safe_getenv("PCM_DEBUG_PMU_DISCOVERY") == std::string("1")); + + size_t socket = 0; - auto processTables = [this, &debug](const uint64 bar, const VSEC & vsec) + auto processTables = [this, &debug, &socket](const uint64 bar, const VSEC & vsec) { try { DBG(2, "Uncore discovery detection. Reading from bar 0x", std::hex, bar, std::dec); @@ -28,7 +30,8 @@ UncorePMUDiscovery::UncorePMUDiscovery() }; UncoreGlobalDiscovery global; mmio_memcpy(global.table, bar, UncoreDiscoverySize * sizeof(uint64), true); - globalPMUs.push_back(global.pmu); + globalPMUs.resize(socket + 1); + globalPMUs[socket].push_back(global.pmu); if (debug) { std::cerr << "Read global.pmu from 0x" << std::hex << bar << std::dec << "\n"; @@ -63,7 +66,9 @@ UncorePMUDiscovery::UncorePMUDiscovery() // unit.pmu.print(); boxPMUMap[unit.pmu.boxType].push_back(unit.pmu); } - boxPMUs.push_back(boxPMUMap); + boxPMUs.resize(socket + 1); + boxPMUs[socket].push_back(boxPMUMap); + ++socket; } catch (const std::exception & e) { @@ -100,18 +105,21 @@ UncorePMUDiscovery::UncorePMUDiscovery() { for (size_t s = 0; s < boxPMUs.size(); ++s) { - std::cout << "Socket " << s << " global PMU:\n"; - std::cout << " "; - globalPMUs[s].print(); - std::cout << "Socket " << s << " unit PMUs:\n"; - for (const auto & pmuType : boxPMUs[s]) + for (size_t die = 0; die < boxPMUs[s].size(); ++die) { - const auto n = pmuType.second.size(); - std::cout << " PMU type " << pmuType.first << " (" << n << " boxes)"<< "\n"; - for (size_t i = 0; i < n ; ++i) + std::cout << "Socket " << s << " die " << die << " global PMU:\n"; + std::cout << " "; + globalPMUs[s][die].print(); + std::cout << "Socket " << s << " die " << die << " unit PMUs:\n"; + for (const auto& pmuType : boxPMUs[s][die]) { - std::cout << " "; - pmuType.second[i].print(); + const auto n = pmuType.second.size(); + std::cout << " PMU type " << pmuType.first << " (" << n << " boxes)" << "\n"; + for (size_t i = 0; i < n; ++i) + { + std::cout << " "; + pmuType.second[i].print(); + } } } } diff --git a/src/uncore_pmu_discovery.h b/src/uncore_pmu_discovery.h index 7caddf7e..e970eaa9 100644 --- a/src/uncore_pmu_discovery.h +++ b/src/uncore_pmu_discovery.h @@ -128,17 +128,17 @@ class UncorePMUDiscovery }; typedef std::vector BoxPMUs; typedef std::unordered_map BoxPMUMap; // boxType -> BoxPMUs - std::vector boxPMUs; - std::vector globalPMUs; + std::vector > boxPMUs; // socket -> die -> BoxPMUs + std::vector > globalPMUs; // socket -> die -> GlobalPMU - bool validBox(const size_t boxType, const size_t socket, const size_t pos) + bool validBox(const size_t boxType, const size_t socket, const size_t die, const size_t pos) { - return socket < boxPMUs.size() && pos < boxPMUs[socket][boxType].size(); + return socket < boxPMUs.size() && die < boxPMUs[socket].size() && pos < boxPMUs[socket][die][boxType].size(); } - size_t registerStep(const size_t boxType, const size_t socket, const size_t pos) + size_t registerStep(const size_t boxType, const size_t socket, const size_t die, const size_t pos) { - const auto width = boxPMUs[socket][boxType][pos].bitWidth; - switch (boxPMUs[socket][boxType][pos].accessType) + const auto width = boxPMUs[socket][die][boxType][pos].bitWidth; + switch (boxPMUs[socket][die][boxType][pos].accessType) { case MSR: if (width <= 64) @@ -171,57 +171,66 @@ class UncorePMUDiscovery public: UncorePMUDiscovery(); - size_t getNumBoxes(const size_t boxType, const size_t socket) + size_t getNumDies(const size_t socket) const { if (socket < boxPMUs.size()) { - return boxPMUs[socket][boxType].size(); + return boxPMUs[socket].size(); } return 0; } - uint64 getBoxCtlAddr(const size_t boxType, const size_t socket, const size_t pos) + size_t getNumBoxes(const size_t boxType, const size_t socket, const size_t die) { - if (validBox(boxType, socket, pos)) + if (socket < boxPMUs.size() && die < boxPMUs[socket].size()) { - return boxPMUs[socket][boxType][pos].boxCtrlAddr; + return boxPMUs[socket][die][boxType].size(); } return 0; } - uint64 getBoxCtlAddr(const size_t boxType, const size_t socket, const size_t pos, const size_t c) + uint64 getBoxCtlAddr(const size_t boxType, const size_t socket, const size_t die, const size_t pos) { - if (validBox(boxType, socket, pos) && c < boxPMUs[socket][boxType][pos].numRegs) + if (validBox(boxType, socket, die, pos)) { - const size_t step = (boxType == SPR_IMC_BOX_TYPE) ? 4 : registerStep(boxType, socket, pos); - return boxPMUs[socket][boxType][pos].boxCtrlAddr + boxPMUs[socket][boxType][pos].ctrlOffset + c * step; + return boxPMUs[socket][die][boxType][pos].boxCtrlAddr; } return 0; } - uint64 getBoxCtrAddr(const size_t boxType, const size_t socket, const size_t pos, const size_t c) + uint64 getBoxCtlAddr(const size_t boxType, const size_t socket, const size_t die, const size_t pos, const size_t c) { - if (validBox(boxType, socket, pos) && c < boxPMUs[socket][boxType][pos].numRegs) + if (validBox(boxType, socket, die, pos) && c < boxPMUs[socket][die][boxType][pos].numRegs) { - return boxPMUs[socket][boxType][pos].boxCtrlAddr + boxPMUs[socket][boxType][pos].ctrOffset + c * registerStep(boxType, socket, pos); + const size_t step = (boxType == SPR_IMC_BOX_TYPE) ? 4 : registerStep(boxType, socket, die, pos); + return boxPMUs[socket][die][boxType][pos].boxCtrlAddr + boxPMUs[socket][die][boxType][pos].ctrlOffset + c * step; } return 0; } - accessTypeEnum getBoxAccessType(const size_t boxType, const size_t socket, const size_t pos) + uint64 getBoxCtrAddr(const size_t boxType, const size_t socket, const size_t die, const size_t pos, const size_t c) { - if (validBox(boxType, socket, pos)) + if (validBox(boxType, socket, die, pos) && c < boxPMUs[socket][die][boxType][pos].numRegs) { - return static_cast(boxPMUs[socket][boxType][pos].accessType); + return boxPMUs[socket][die][boxType][pos].boxCtrlAddr + boxPMUs[socket][die][boxType][pos].ctrOffset + c * registerStep(boxType, socket, die, pos); + } + return 0; + } + + accessTypeEnum getBoxAccessType(const size_t boxType, const size_t socket, const size_t die, const size_t pos) + { + if (validBox(boxType, socket, die, pos)) + { + return static_cast(boxPMUs[socket][die][boxType][pos].accessType); } return unknownAccessType; } - uint64 getBoxNumRegs(const size_t boxType, const size_t socket, const size_t pos) + uint64 getBoxNumRegs(const size_t boxType, const size_t socket, const size_t die, const size_t pos) { - if (validBox(boxType, socket, pos)) + if (validBox(boxType, socket, die, pos)) { - return boxPMUs[socket][boxType][pos].numRegs; + return boxPMUs[socket][die][boxType][pos].numRegs; } return 0; } From 07ee683a2734d44ebf13d0b8908c64c5430902dc Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Tue, 20 Jan 2026 18:43:34 +0100 Subject: [PATCH 07/19] Add compiler security hardening flags Linux: -fstack-protector-strong, -z,relro, -z,now, -z,noexecstack Windows: /GS, /Gy, /DYNAMICBASE, /NXCOMPAT, /Qspectre, /W4 --- CMakeLists.txt | 6 +++--- src/CMakeLists.txt | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1bdc5063..73f81465 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,9 +67,9 @@ if(UNIX) # APPLE, LINUX, FREE_BSD set (PCM_COMMON_FLAGS "-Wno-unknown-pragmas -DCMAKE_INSTALL_PREFIX=\"${CMAKE_INSTALL_PREFIX}\"") if(LINUX) - set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wextra -DPCM_USE_PERF") - if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") - set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wl,-z,now") + set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wextra -DPCM_USE_PERF -fstack-protector-strong") + if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") + set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wl,-z,now -Wl,-z,relro -Wl,-z,noexecstack") endif() if(NOT DEFINED LINUX_SYSTEMD) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 79458f01..c19d30db 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -99,8 +99,14 @@ if(UNIX) # LINUX, FREE_BSD, APPLE endif() if(MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2 /wd4251 /wd4273 /EHa /Zi") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2 /wd4251 /wd4273 /EHa /Zi /GS /Gy") add_definitions(/W3) + # SDL429 security flags: ASLR (/DYNAMICBASE) and DEP (/NXCOMPAT) + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /DYNAMICBASE /NXCOMPAT") + # Optional Spectre mitigation for newer MSVC versions + if(MSVC_TOOLSET_VERSION GREATER_EQUAL 141) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Qspectre") + endif() # windows/* files -> PCM_STATIC file(GLOB WINDOWS_SOURCES winpmem/winpmem.cpp windows/stdafx.cpp freegetopt/getopt.cpp) From c280c05f94ddfa4ef28e715f6be510335a16d14e Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Fri, 9 Jan 2026 19:29:12 +0100 Subject: [PATCH 08/19] SDL438: follow Bandit guidance Fix 4 HIGH severity command injection vulnerabilities (B602/CWE-78) in pmu-query.py --- scripts/pmu-query.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pmu-query.py b/scripts/pmu-query.py index 9a980081..8f9fcf5c 100644 --- a/scripts/pmu-query.py +++ b/scripts/pmu-query.py @@ -46,17 +46,17 @@ break if platform.system() == "CYGWIN_NT-6.1": - p = subprocess.Popen(["./pcm-core.exe -c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["./pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=False) elif platform.system() == "Windows": - p = subprocess.Popen(["pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=False) elif platform.system() == "Linux": pcm_core = shutil.which("pcm-core") if not pcm_core: print("Could not find pcm-core executable!") sys.exit(-1) - p = subprocess.Popen([pcm_core, "-c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen([pcm_core, "-c"], stdout=subprocess.PIPE, shell=False) else: - p = subprocess.Popen(["../build/bin/pcm-core -c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["../build/bin/pcm-core", "-c"], stdout=subprocess.PIPE, shell=False) (output, err) = p.communicate() p_status = p.wait() From fd65f86e8b1edd99bb729e5afcc8df69a634e738 Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Wed, 21 Jan 2026 09:03:40 +0100 Subject: [PATCH 09/19] Protect software against DLL injections (for WinRing0 dlls) Usage of LoadLibraryEx function with LOAD_LIBRARY_SEARCH_SYSTEM32 is added --- src/winring0/OlsApiInit.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/winring0/OlsApiInit.h b/src/winring0/OlsApiInit.h index fd9383b0..da67ab04 100644 --- a/src/winring0/OlsApiInit.h +++ b/src/winring0/OlsApiInit.h @@ -135,13 +135,22 @@ _SetOlsValue SetOlsValue = NULL; BOOL InitOpenLibSys(HMODULE *hModule) { TCHAR dll_path[MAX_PATH]; - GetSystemDirectory(dll_path, MAX_PATH - 20); + // SDL436: Use fully qualified path from System32 directory + // GetSystemDirectory ensures we load from the trusted System32 location + if (!GetSystemDirectory(dll_path, MAX_PATH - 20)) + { + std::wcerr << "Failed to get System32 directory path.\n"; + return FALSE; + } #ifdef _M_X64 _tcscat_s(dll_path, MAX_PATH, TEXT("\\WinRing0x64.dll")); #else _tcscat_s(dll_path, MAX_PATH, TEXT("\\WinRing0.dll")); #endif - *hModule = LoadLibrary(dll_path); + + // SDL436: Use LoadLibraryEx with LOAD_LIBRARY_SEARCH_SYSTEM32 flag + // This ensures secure DLL loading from System32 only, preventing DLL injection + *hModule = LoadLibraryEx(dll_path, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); if(*hModule == NULL) { From d2537bb5550cd5a9488bb02f282bf3b211de84fa Mon Sep 17 00:00:00 2001 From: "Dementiev, Roman" Date: Wed, 21 Jan 2026 15:16:57 +0100 Subject: [PATCH 10/19] dump more fields in the dvsec header Change-Id: Ib7b72f15a9da50ecda149ed7611e6c076969c369 --- src/pci.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pci.h b/src/pci.h index a998454c..0007aadd 100644 --- a/src/pci.h +++ b/src/pci.h @@ -231,7 +231,9 @@ void processDVSEC(MatchFunc matchFunc, ProcessFunc processFunc) { return; } - DBG(2, "offset 0x" , std::hex , offset , " cap_id: 0x" , header.fields.cap_id , " vsec_id: 0x", header.fields.vsec_id, " entryID: 0x" , std::hex , header.fields.entryID , std::dec); + DBG(2, "offset 0x" , std::hex , offset , " cap_id: 0x" , header.fields.cap_id , " vsec_id: 0x", header.fields.vsec_id, " entryID: 0x" , std::hex , header.fields.entryID , + " NumEntries: ", std::dec, header.fields.NumEntries, " EntrySize: ", std::dec, header.fields.EntrySize, " tBIR: ", header.fields.tBIR, + " Address: 0x", std::hex, header.fields.Address, std::dec); if (matchFunc(header)) { DBG(2, ".... found match. tBIR = ", header.fields.tBIR); From d1c0b3221595f8e31c9d261215574e3d9496d151 Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Fri, 9 Jan 2026 18:48:16 +0100 Subject: [PATCH 11/19] SDL330: Protect critical file operations against symlink attacks Add O_NOFOLLOW flags and symlink validation to MSR, PCI, SysFS, and daemon file operations to prevent privilege escalation via symlink redirects. --- src/daemon/daemon.cpp | 7 +++++++ src/msr.cpp | 16 ++++++++++++---- src/pci.cpp | 40 +++++++++++++++++++++++++++++++--------- src/utils.cpp | 15 ++++++++++++++- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 0a0bee20..4dde48f6 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -314,6 +314,13 @@ namespace PCMDaemon { } //Store shm id in a file (shmIdLocation_) + // SDL330: Check for symlinks BEFORE attempting to remove + struct stat st; + if (lstat(shmIdLocation_.c_str(), &st) == 0 && S_ISLNK(st.st_mode)) { + std::cerr << "SDL330 CRITICAL: Symlink detected at shared memory id location: " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); + } + int success = remove(shmIdLocation_.c_str()); if (success != 0) { diff --git a/src/msr.cpp b/src/msr.cpp index cdbfc433..7b4ef0f5 100644 --- a/src/msr.cpp +++ b/src/msr.cpp @@ -166,8 +166,13 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) { char path[200]; snprintf(path, 200, "/dev/cpuctl%u", cpu); - int handle = ::open(path, O_RDWR); - if (handle < 0) throw std::exception(); + int handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at " << path << "\n"; + } + throw std::exception(); + } fd = handle; } @@ -229,11 +234,14 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) char * path = new char[200]; if (!path) throw std::runtime_error("Allocation of 200 bytes failed."); snprintf(path, 200, "/dev/cpu/%u/msr", cpu); - int handle = ::open(path, O_RDWR); + int handle = ::open(path, O_RDWR | O_NOFOLLOW); if (handle < 0) { // try Android msr device path snprintf(path, 200, "/dev/msr%u", cpu); - handle = ::open(path, O_RDWR); + handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0 && errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at MSR device path\n"; + } } deleteAndNullifyArray(path); if (handle < 0) diff --git a/src/pci.cpp b/src/pci.cpp index 89a852ab..86c02efc 100644 --- a/src/pci.cpp +++ b/src/pci.cpp @@ -258,8 +258,13 @@ PciHandle::PciHandle(uint32 groupnr_, uint32 bus_, uint32 device_, uint32 functi device(device_), function(function_) { - int handle = ::open("/dev/pci", O_RDWR); - if (handle < 0) throw std::exception(); + int handle = ::open("/dev/pci", O_RDWR | O_NOFOLLOW); + if (handle < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at /dev/pci\n"; + } + throw std::exception(); + } fd = handle; } @@ -271,8 +276,13 @@ bool PciHandle::exists(uint32 groupnr_, uint32 bus_, uint32 device_, uint32 func int fd; int ret; - fd = ::open("/dev/pci", O_RDWR, 0); - if (fd < 0) return false; + fd = ::open("/dev/pci", O_RDWR | O_NOFOLLOW, 0); + if (fd < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at /dev/pci\n"; + } + return false; + } bzero(&pc, sizeof(pc)); @@ -391,11 +401,15 @@ int openHandle(uint32 groupnr_, uint32 bus, uint32 device, uint32 function) // std::cout << "PciHandle: Opening "< parseBitsParameter(const char * param) #ifdef __linux__ FILE * tryOpen(const char * path, const char * mode) { + // SDL330: Check for symlinks before opening + struct stat st; + if (lstat(path, &st) == 0 && S_ISLNK(st.st_mode)) { + std::cerr << "SDL330 SECURITY: Symlink detected at " << path << " - rejecting file access\n"; + return nullptr; + } + FILE * f = fopen(path, mode); if (!f) { - f = fopen((std::string("/pcm") + path).c_str(), mode); + const std::string alt_path = std::string("/pcm") + path; + // SDL330: Check for symlinks on alternate path too + if (lstat(alt_path.c_str(), &st) == 0 && S_ISLNK(st.st_mode)) { + std::cerr << "SDL330 SECURITY: Symlink detected at " << alt_path << " - rejecting file access\n"; + return nullptr; + } + f = fopen(alt_path.c_str(), mode); } return f; } From df3967bc90c3f6fb6e3ba9f87979c58cce410257 Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Tue, 20 Jan 2026 22:38:53 +0100 Subject: [PATCH 12/19] Fix TOCTOU vulnerabilities for SDL330 symlink protection - daemon.cpp: Use unlink()+open(O_EXCL|O_NOFOLLOW) in setupSharedMemory (CID 8073804) - utils.cpp: Use open(O_NOFOLLOW)+fdopen() in tryOpen (CID 8073803) --- src/daemon/daemon.cpp | 29 ++++++++++++++++++----------- src/utils.cpp | 34 ++++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 4dde48f6..b8e762af 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -313,24 +314,30 @@ namespace PCMDaemon { exit(EXIT_FAILURE); } - //Store shm id in a file (shmIdLocation_) - // SDL330: Check for symlinks BEFORE attempting to remove - struct stat st; - if (lstat(shmIdLocation_.c_str(), &st) == 0 && S_ISLNK(st.st_mode)) { - std::cerr << "SDL330 CRITICAL: Symlink detected at shared memory id location: " << shmIdLocation_ << "\n"; - exit(EXIT_FAILURE); + // Store shm id in a file (shmIdLocation_) + int success = unlink(shmIdLocation_.c_str()); + if (success != 0 && errno != ENOENT) + { + std::cerr << "Failed to delete shared memory id location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; } - int success = remove(shmIdLocation_.c_str()); - if (success != 0) + // SDL330: atomic file creation with symlink protection + int fd = open(shmIdLocation_.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW, 0660); + if (fd < 0) { - std::cerr << "Failed to delete shared memory id location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + if (errno == EEXIST) { + std::cerr << "SDL330 CRITICAL: File unexpectedly exists at shared memory id location (possible race condition/symlink attack): " << shmIdLocation_ << "\n"; + } else { + std::cerr << "Failed to create shared memory key location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + } + exit(EXIT_FAILURE); } - FILE* fp = fopen(shmIdLocation_.c_str(), "w"); + FILE* fp = fdopen(fd, "w"); if (!fp) { - std::cerr << "Failed to create/write to shared memory key location: " << shmIdLocation_ << "\n"; + close(fd); + std::cerr << "Failed to open stream for shared memory key location: " << shmIdLocation_ << "\n"; exit(EXIT_FAILURE); } fprintf(fp, "%i", sharedMemoryId_); diff --git a/src/utils.cpp b/src/utils.cpp index 5a42ba56..64aac699 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -17,6 +17,7 @@ #else #include // for waitpid() #include // for ::sleep +#include // for open(), O_NOFOLLOW #endif #include "utils.h" #include "cpucounters.h" @@ -1500,23 +1501,28 @@ std::pair parseBitsParameter(const char * param) #ifdef __linux__ FILE * tryOpen(const char * path, const char * mode) { - // SDL330: Check for symlinks before opening - struct stat st; - if (lstat(path, &st) == 0 && S_ISLNK(st.st_mode)) { - std::cerr << "SDL330 SECURITY: Symlink detected at " << path << " - rejecting file access\n"; - return nullptr; + // SDL330: O_NOFOLLOW rejects symlinks atomically (no TOCTOU) + int flags = O_NOFOLLOW; + if (strchr(mode, 'w')) { + flags |= O_WRONLY | O_CREAT | O_TRUNC; + } else { + flags |= O_RDONLY; } - - FILE * f = fopen(path, mode); - if (!f) + + int fd = open(path, flags, 0644); + if (fd < 0) { const std::string alt_path = std::string("/pcm") + path; - // SDL330: Check for symlinks on alternate path too - if (lstat(alt_path.c_str(), &st) == 0 && S_ISLNK(st.st_mode)) { - std::cerr << "SDL330 SECURITY: Symlink detected at " << alt_path << " - rejecting file access\n"; - return nullptr; - } - f = fopen(alt_path.c_str(), mode); + fd = open(alt_path.c_str(), flags, 0644); + } + + if (fd < 0) { + return nullptr; + } + + FILE * f = fdopen(fd, mode); + if (!f) { + close(fd); } return f; } From a603426e449555dc073c4799368433d5bc4e441d Mon Sep 17 00:00:00 2001 From: Roman Dementiev Date: Wed, 21 Jan 2026 15:38:54 +0100 Subject: [PATCH 13/19] Update src/msr.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/msr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msr.cpp b/src/msr.cpp index 7b4ef0f5..c445e522 100644 --- a/src/msr.cpp +++ b/src/msr.cpp @@ -240,7 +240,7 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) snprintf(path, 200, "/dev/msr%u", cpu); handle = ::open(path, O_RDWR | O_NOFOLLOW); if (handle < 0 && errno == ELOOP) { - std::cerr << "SDL330 ERROR: Symlink detected at MSR device path\n"; + std::cerr << "SDL330 ERROR: Symlink detected at MSR device path " << path << "\n"; } } deleteAndNullifyArray(path); From 110ccba94a252150328ec6af59142f40e8d85617 Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Wed, 21 Jan 2026 21:32:28 +0100 Subject: [PATCH 14/19] Eliminate TOCTOU race with O_EXCL-first retry loop in daemon Replace upfront unlink() with atomic O_EXCL attempt first, retry on EEXIST with max 3 attempts to prevent both race conditions and DoS. --- src/daemon/daemon.cpp | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index b8e762af..90eccb5d 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -315,21 +315,31 @@ namespace PCMDaemon { } // Store shm id in a file (shmIdLocation_) - int success = unlink(shmIdLocation_.c_str()); - if (success != 0 && errno != ENOENT) - { - std::cerr << "Failed to delete shared memory id location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; - } - - // SDL330: atomic file creation with symlink protection - int fd = open(shmIdLocation_.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW, 0660); - if (fd < 0) - { + // SDL330: Atomic file creation with symlink protection + // Try O_EXCL first, unlink and retry only if needed (avoids TOCTOU race) + int fd = -1; + for (int attempt = 0; attempt < 3 && fd < 0; ++attempt) { + fd = open(shmIdLocation_.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW, 0660); + if (fd >= 0) break; + + if (errno == ELOOP) { + std::cerr << "SDL330 CRITICAL: Symlink detected at " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); + } if (errno == EEXIST) { - std::cerr << "SDL330 CRITICAL: File unexpectedly exists at shared memory id location (possible race condition/symlink attack): " << shmIdLocation_ << "\n"; - } else { - std::cerr << "Failed to create shared memory key location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + // File exists from previous run - unlink and retry + if (unlink(shmIdLocation_.c_str()) != 0 && errno != ENOENT) { + std::cerr << "Failed to delete stale shared memory id file: " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); + } + continue; // retry } + // Other error + std::cerr << "Failed to create shared memory key location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + exit(EXIT_FAILURE); + } + if (fd < 0) { + std::cerr << "SDL330 CRITICAL: Unable to create shared memory file after retries (possible attack): " << shmIdLocation_ << "\n"; exit(EXIT_FAILURE); } From 2f46ce802e05ed8262114231d0d45b27f0b39b1d Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Wed, 21 Jan 2026 22:03:42 +0100 Subject: [PATCH 15/19] SDL330: Add ELOOP check for /dev/cpu/*/msr and define retry constant --- src/daemon/daemon.cpp | 7 ++++--- src/msr.cpp | 3 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 90eccb5d..d64318d7 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -318,7 +318,8 @@ namespace PCMDaemon { // SDL330: Atomic file creation with symlink protection // Try O_EXCL first, unlink and retry only if needed (avoids TOCTOU race) int fd = -1; - for (int attempt = 0; attempt < 3 && fd < 0; ++attempt) { + constexpr int MAX_FILE_CREATION_RETRIES = 3; + for (int attempt = 0; attempt < MAX_FILE_CREATION_RETRIES && fd < 0; ++attempt) { fd = open(shmIdLocation_.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW, 0660); if (fd >= 0) break; @@ -361,7 +362,7 @@ namespace PCMDaemon { shmData.shm_perm.gid = gid; shmData.shm_perm.mode = mode; - success = shmctl(sharedMemoryId_, IPC_SET, &shmData); + int success = shmctl(sharedMemoryId_, IPC_SET, &shmData); if (success < 0) { std::cerr << "Failed to IPC_SET (errno=" << errno << ")\n"; @@ -773,7 +774,7 @@ namespace PCMDaemon { else { // Delete segment - success = shmctl(sharedMemoryId_, IPC_RMID, NULL); + int success = shmctl(sharedMemoryId_, IPC_RMID, NULL); if (success != 0) { std::cerr << "Failed to delete the shared memory segment (errno=" << errno << ")\n"; diff --git a/src/msr.cpp b/src/msr.cpp index c445e522..2709c5b0 100644 --- a/src/msr.cpp +++ b/src/msr.cpp @@ -235,6 +235,9 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) if (!path) throw std::runtime_error("Allocation of 200 bytes failed."); snprintf(path, 200, "/dev/cpu/%u/msr", cpu); int handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0 && errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at MSR device path " << path << "\n"; + } if (handle < 0) { // try Android msr device path snprintf(path, 200, "/dev/msr%u", cpu); From c4afa303b9717af223f6745896d4548de8a9e3c7 Mon Sep 17 00:00:00 2001 From: Roman Dementiev Date: Thu, 22 Jan 2026 14:37:06 +0100 Subject: [PATCH 16/19] tpmi: the vsec address is in 8-byte units --- src/tpmi.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tpmi.cpp b/src/tpmi.cpp index 07aacfda..81c6b129 100644 --- a/src/tpmi.cpp +++ b/src/tpmi.cpp @@ -57,7 +57,7 @@ class PFSInstances assert(vsec.fields.EntrySize == 2); std::vector pfsArray(vsec.fields.NumEntries); try { - mmio_memcpy(&(pfsArray[0]), bar + vsec.fields.Address, vsec.fields.NumEntries * sizeof(PFS), true, true); + mmio_memcpy(&(pfsArray[0]), bar + 8ULL * vsec.fields.Address, vsec.fields.NumEntries * sizeof(PFS), true, true); } catch (std::runtime_error & e) { std::cerr << "Can't read PFS\n"; @@ -79,7 +79,7 @@ class PFSInstances for (uint64 p = 0; p < pfs.NumEntries; ++p) { uint32 reg0 = 0; - const auto addr = bar + vsec.fields.Address + pfs.CapOffset * 1024ULL + p * pfs.EntrySize * sizeof(uint32); + const auto addr = bar + 8ULL * vsec.fields.Address + pfs.CapOffset * 1024ULL + p * pfs.EntrySize * sizeof(uint32); try { mmio_memcpy(®0, addr, sizeof(uint32), false, true); } catch (std::runtime_error & e) From ad80d93c1b35a0f94f6e36df11d57c1a9ca0121d Mon Sep 17 00:00:00 2001 From: Thomas Willhalm Date: Fri, 23 Jan 2026 10:24:56 +0100 Subject: [PATCH 17/19] typos fixed --- src/pcm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pcm.cpp b/src/pcm.cpp index 7205062e..ba708be7 100644 --- a/src/pcm.cpp +++ b/src/pcm.cpp @@ -186,7 +186,7 @@ void print_output(PCM * m, cout << " IPC : instructions per CPU cycle\n"; if (m->isActiveRelativeFrequencyAvailable()) { - cout << " CFREQ : core frequency in Ghz\n"; + cout << " CFREQ : core frequency in GHz\n"; } break; default: @@ -1651,4 +1651,4 @@ int mainThrows(int argc, char * argv[]) exit(EXIT_SUCCESS); } -#endif // UNIT_TEST \ No newline at end of file +#endif // UNIT_TEST From effd2dc199911314b714619d86f884ce75ef7ba8 Mon Sep 17 00:00:00 2001 From: Maria Markova Date: Fri, 23 Jan 2026 10:28:37 +0100 Subject: [PATCH 18/19] Add Bandit security scan workflow - Run Bandit only when Python files or config changes - Exclude submodules from scan - Fail CI on any security issues detected --- .bandit/bandit.config | 394 ++++++++++++++++++++++++++++++++ .github/workflows/ci-bandit.yml | 35 +++ scripts/pcm-win-power-tray.py | 2 +- 3 files changed, 430 insertions(+), 1 deletion(-) create mode 100644 .bandit/bandit.config create mode 100644 .github/workflows/ci-bandit.yml diff --git a/.bandit/bandit.config b/.bandit/bandit.config new file mode 100644 index 00000000..8d0c07f3 --- /dev/null +++ b/.bandit/bandit.config @@ -0,0 +1,394 @@ +### This config may optionally select a subset of tests to run or skip by +### filling out the 'tests' and 'skips' lists given below. If no tests are +### specified for inclusion then it is assumed all tests are desired. The skips +### set will remove specific tests from the include set. This can be controlled +### using the -t/-s CLI options. Note that the same test ID should not appear +### in both 'tests' and 'skips', this would be nonsensical and is detected by +### Bandit at runtime. + +# Available tests: +# B101 : assert_used +# B102 : exec_used +# B103 : set_bad_file_permissions +# B104 : hardcoded_bind_all_interfaces +# B105 : hardcoded_password_string +# B106 : hardcoded_password_funcarg +# B107 : hardcoded_password_default +# B108 : hardcoded_tmp_directory +# B110 : try_except_pass +# B112 : try_except_continue +# B201 : flask_debug_true +# B301 : pickle +# B302 : marshal +# B303 : md5 +# B304 : ciphers +# B305 : cipher_modes +# B306 : mktemp_q +# B307 : eval +# B308 : mark_safe +# B310 : urllib_urlopen +# B311 : random +# B312 : telnetlib +# B313 : xml_bad_cElementTree +# B314 : xml_bad_ElementTree +# B315 : xml_bad_expatreader +# B316 : xml_bad_expatbuilder +# B317 : xml_bad_sax +# B318 : xml_bad_minidom +# B319 : xml_bad_pulldom +# B321 : ftplib +# B323 : unverified_context +# B324 : hashlib_new_insecure_functions +# B401 : import_telnetlib +# B402 : import_ftplib +# B403 : import_pickle +# B404 : import_subprocess +# B405 : import_xml_etree +# B406 : import_xml_sax +# B407 : import_xml_expat +# B408 : import_xml_minidom +# B409 : import_xml_pulldom +# B411 : import_xmlrpclib +# B412 : import_httpoxy +# B413 : import_pycrypto +# B501 : request_with_no_cert_validation +# B502 : ssl_with_bad_version +# B503 : ssl_with_bad_defaults +# B504 : ssl_with_no_version +# B505 : weak_cryptographic_key +# B506 : yaml_load +# B507 : ssh_no_host_key_verification +# B601 : paramiko_calls +# B602 : subprocess_popen_with_shell_equals_true +# B603 : subprocess_without_shell_equals_true +# B604 : any_other_function_with_shell_equals_true +# B605 : start_process_with_a_shell +# B606 : start_process_with_no_shell +# B607 : start_process_with_partial_path +# B608 : hardcoded_sql_expressions +# B609 : linux_commands_wildcard_injection +# B610 : django_extra_used +# B611 : django_rawsql_used +# B701 : jinja2_autoescape_false +# B702 : use_of_mako_templates +# B703 : django_mark_safe + +# (optional) list included test IDs here, eg '[B101, B406]': +# Required security checkers - do not disable these +# Additional checkers may be added if desired +tests: + [ 'B301', 'B302', 'B303', 'B304', 'B305', 'B306', 'B308', 'B310', 'B311', 'B312', 'B313', 'B314', 'B315', 'B316', 'B317', 'B318', 'B319', 'B321', 'B323', 'B324', 'B401', 'B402', 'B403', 'B404', 'B405', 'B406', 'B407', 'B408', 'B409', 'B411', 'B412', 'B413'] + +# (optional) list skipped test IDs here, eg '[B101, B406]': +# Optional checkers - may be added to tests list if desired +skips: + [ 'B101', 'B102', 'B103', 'B104', 'B105', 'B106', 'B107', 'B108', 'B110', 'B112', 'B201', 'B501', 'B502', 'B503', 'B504', 'B505', 'B506', 'B507', 'B601', 'B602', 'B603', 'B604', 'B605', 'B606', 'B607', 'B608', 'B609', 'B610', 'B611', 'B701', 'B702', 'B703'] + +### (optional) plugin settings - some test plugins require configuration data +### that may be given here, per-plugin. All bandit test plugins have a built in +### set of sensible defaults and these will be used if no configuration is +### provided. It is not necessary to provide settings for every (or any) plugin +### if the defaults are acceptable. + +any_other_function_with_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +assert_used: + skips: [] +hardcoded_tmp_directory: + tmp_dirs: + - /tmp + - /var/tmp + - /dev/shm +linux_commands_wildcard_injection: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +ssl_with_bad_defaults: + bad_protocol_versions: + - PROTOCOL_SSLv2 + - SSLv2_METHOD + - SSLv23_METHOD + - PROTOCOL_SSLv3 + - PROTOCOL_TLSv1 + - SSLv3_METHOD + - TLSv1_METHOD +ssl_with_bad_version: + bad_protocol_versions: + - PROTOCOL_SSLv2 + - SSLv2_METHOD + - SSLv23_METHOD + - PROTOCOL_SSLv3 + - PROTOCOL_TLSv1 + - SSLv3_METHOD + - TLSv1_METHOD +start_process_with_a_shell: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +start_process_with_no_shell: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +start_process_with_partial_path: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +subprocess_popen_with_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +subprocess_without_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +try_except_continue: + check_typed_exception: false +try_except_pass: + check_typed_exception: false +weak_cryptographic_key: + weak_key_size_dsa_high: 1024 + weak_key_size_dsa_medium: 2048 + weak_key_size_ec_high: 160 + weak_key_size_ec_medium: 224 + weak_key_size_rsa_high: 1024 + weak_key_size_rsa_medium: 2048 diff --git a/.github/workflows/ci-bandit.yml b/.github/workflows/ci-bandit.yml new file mode 100644 index 00000000..f27864ce --- /dev/null +++ b/.github/workflows/ci-bandit.yml @@ -0,0 +1,35 @@ +name: bandit-security-scan + +on: + push: + branches: [ '**' ] + paths: + - '**.py' + - '.bandit/**' + - '.github/workflows/ci-bandit.yml' + pull_request: + branches: [ '**' ] + paths: + - '**.py' + - '.bandit/**' + - '.github/workflows/ci-bandit.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + bandit: + runs-on: ci-test + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 + with: + python-version: '3.12' + + - name: Install and run Bandit + run: | + pip install bandit + bandit -r . -c .bandit/bandit.config -x ./Intel-PMT,./perfmon,./src/pugixml,./src/simdjson,./tests/googletest -f txt diff --git a/scripts/pcm-win-power-tray.py b/scripts/pcm-win-power-tray.py index 84fa7ed1..f0b59807 100644 --- a/scripts/pcm-win-power-tray.py +++ b/scripts/pcm-win-power-tray.py @@ -2,7 +2,7 @@ from PIL import Image, ImageDraw, ImageFont import threading import time -import subprocess +import subprocess # nosec B404 - subprocess used for controlled system commands, no user input import csv import io From 4803bfc84ad0071867d9bf7122e534c8b42d743b Mon Sep 17 00:00:00 2001 From: "Dementiev, Roman" Date: Fri, 23 Jan 2026 15:22:47 +0100 Subject: [PATCH 19/19] add CI repo restriction Change-Id: I883bf8ae288e2d1fda359eff0f4ce4f39901dba5 --- .github/workflows/ci-bandit.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci-bandit.yml b/.github/workflows/ci-bandit.yml index f27864ce..07a9fd38 100644 --- a/.github/workflows/ci-bandit.yml +++ b/.github/workflows/ci-bandit.yml @@ -21,6 +21,7 @@ permissions: jobs: bandit: runs-on: ci-test + if: ${{ github.repository != 'intel/pcm' }} steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2