Fix privileged fd leak in libpqos and symlink race in pqos safe_fopen by camuso · Pull Request #312 · intel/intel-cmt-cat

camuso · 2026-08-31T18:46:17Z

Two security issues exist in intel-cmt-cat that allow local privilege
escalation or data corruption when the tools are exposed to non-root
users through sudo or similar delegation:

libpqos opens MSR device files and the API lock file without
O_CLOEXEC. These long-lived descriptors survive execve() and
can be inherited by child processes that should not have access to
MSR registers or the API lock.
safe_fopen() in pqos is vulnerable to symlink-following file
truncation. The current implementation calls lstat() to check
for symlinks, then fopen() to open the file. With mode "w+",
fopen() follows a pre-existing symlink and truncates the target
before the post-open lstat/fstat comparison can detect the
mismatch. No race condition is required — a pre-existing symlink
is sufficient to truncate an arbitrary file.

The series is structured as follows:

Commit 1/2 adds O_CLOEXEC to long-lived descriptor opens in
lib/machine.c (MSR device files) and lib/lock.c (API lock file)
as defense-in-depth. These descriptors will be automatically closed
on any execve() call.
Commit 2/2 replaces the lstat()+fopen() pattern in
pqos/common.c:safe_fopen() with open(O_NOFOLLOW)+fdopen().
This eliminates the symlink vulnerability entirely — the kernel
rejects symlinks atomically at open time with ELOOP, so there is
no window for truncation to occur.

Note: rdtset (removed in v26.06) also leaked inherited MSR file
descriptors to child processes after privilege drop. That issue is
no longer present on master since rdtset has been removed.

Tested on RHEL-10 and RHEL-9 with intel-cmt-cat 26.03 on Intel Xeon
Gold and Dell PowerEdge R660 platforms. The symlink test confirms
that on unpatched code, fopen("w+") truncates the symlink target;
on patched code, O_NOFOLLOW rejects it with ELOOP.

rkanagar · 2026-09-09T13:13:34Z

Thanks @camuso . I will merge this before next release.

Copilot

🟡 Changes recommended

Symlinked parent components remain exploitable, file permissions regress, and existing lock tests will fail.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens privileged file descriptors and safe_fopen() against inheritance and symlink attacks.

Changes:

Adds O_CLOEXEC to MSR and API-lock descriptors.
Replaces lstat()/fopen() with open(O_NOFOLLOW)/fdopen().

File summaries

File	Description
`pqos/common.c`	Atomically rejects final-component symlinks.
`lib/machine.c`	Prevents MSR descriptor inheritance.
`lib/lock.c`	Prevents API-lock descriptor inheritance.

Review details

Files reviewed: 3/3 changed files
Comments generated: 4
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        else
+                return NULL;
+
+        fd = open(name, flags | O_NOFOLLOW, perms);


+        m_apilock =
+            open(LOCKFILE, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, LOCKFILE_PERMS);


-                        return NULL;
-                else
-                        new_file = 1;
+        mode_t perms = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH; /* 0644 */


+        fd = open(name, flags | O_NOFOLLOW, perms);
+        if (fd == -1) {
+                if (errno == ELOOP)
+                        printf("File %s is a symlink\n", name);
+                return NULL;


Add O_CLOEXEC to long-lived descriptor opens in lib/machine.c (MSR device files) and lib/lock.c (API lock file) as defense-in-depth against leaking privileged file descriptors across exec boundaries. Without O_CLOEXEC, these descriptors survive execve() and can be inherited by child processes that should not have access to MSR registers or the API lock. Signed-off-by: Tony Camuso <tcamuso@redhat.com>

Replace the lstat()+fopen() pattern in safe_fopen() with open(O_NOFOLLOW)+fdopen(). The original code validates symlinks only after fopen() has already opened and potentially truncated the symlink target. With fopen("w+"), a pre-existing symlink causes the target to be truncated to zero bytes before the post-open lstat/fstat comparison can detect the mismatch. Using open() with O_NOFOLLOW makes the symlink rejection atomic: the kernel returns ELOOP without opening or modifying anything. Signed-off-by: Tony Camuso <tcamuso@redhat.com>

Add a new cmocka test suite exercising safe_fopen() from pqos/common.c: - NULL name / NULL mode / invalid mode → returns NULL - Regular file write+read round-trip - Symlink rejection: safe_fopen through a symlink returns NULL and the target file is not truncated (O_NOFOLLOW) - File permissions: newly created files get 0666 & ~umask, matching fopen() behaviour - Append mode: existing content is preserved The test links only common.o (plus a stub for pqos_get_num_mem_regions) so it is independent of the mock library and builds cleanly with cmocka 2.x. Signed-off-by: Tony Camuso <tcamuso@redhat.com>

camuso · 2026-09-14T13:03:31Z

@rkanagar
Will intel be issuing CVEs for these vulnerabilities, the two originally reported
and the additional one that CoPilot found?
I have devised patches for the additional vulnerability discovered by CoPilot and a fix for the problem introduced by my previous series.

safe_fopen() asked lstat() whether the name was a symbolic link and then handed the name to fopen(), which resolved it a second time. A link already sitting at the name was therefore followed, and the file it pointed at was truncated by the "w+" the monitoring output file uses or created through a dangling link; the comparison of the two stat results that came afterwards could only refuse the stream, never put the file back. No race was needed for that, only a link, and the callers are --config-file, -o/--output-file and a sysfs read. The name is now resolved once: the mode string is translated to open() flags, the file is opened with O_NOFOLLOW, and the stream is made from the descriptor with fdopen(). The kernel refuses a link before anything is opened, created or truncated, which is the same shape safe_open() took for the same reason. Three details are worth naming. Every mode C defines is translated, and where each character sits decides the answer: a first character of 'r', 'w' or 'a', then '+' and 'b' in either order, and last of all the exclusive 'x' that C11 adds for a 'w' - which makes those modes exactly "wx", "wbx", "w+x", "w+bx" and "wb+x", and leaves "wxb" and "wx+" as no more C modes than "rx" is. Everything else is refused with EINVAL. The position matters because a 'b' accepted wherever it appeared would translate "bw" as "w", and the file would be opened and truncated before fdopen() refused the mode - the very thing this function is for. An empty mode is refused before that walk, which starts past the access character and would otherwise read past the end of it. One thing fdopen() does not do for free: it takes the offset the descriptor has, which open() leaves at the start, while fopen() puts a plain append stream at the end of the file. Writes land at the end either way, that being what O_APPEND is for, but a caller that asks ftell() before writing now gets what fopen() would have given it. A target that cannot seek keeps the offset it has, and "a+" is left alone because fopen() positions that one at the start for reading. 'x' becomes O_EXCL and takes O_TRUNC out of the flags, since a file that cannot already exist has nothing to truncate. It is also the one modifier that means nothing to a descriptor that is already open, and glibc's fdopen() refuses "w+x" outright, so the stream is made with the mode without it - by then the exclusion has happened. An exclusive mode does not weaken the symlink refusal: O_CREAT | O_EXCL makes the kernel answer EEXIST for a link rather than ELOOP, and because the name is asked about after the failure rather than the errno being read, the caller is told it is a link either way. What open() reports for a link it refused is not one thing - ELOOP on Linux, EMLINK on FreeBSD, EEXIST under O_EXCL, and ENOENT for a link whose target is missing, which is what "w+" on a dangling link answers - so the failure is explained by asking about the name afterwards, where the answer cannot affect what was opened, and the caller is told what safe_open() tells it. errno is preserved around that and around the close(), since lstat(), printf() and close() may all change it. The descriptor carries O_CLOEXEC, because the stream outlives the call and nothing that inherits it has a use for it. A file the function creates keeps the permissions fopen() gave one, so a file it makes is the file it has always made. unit-test/pqos/test_common.c covers the eleven new cases. Two of them assert the file system rather than the return value, because that is where the defect was: with a link at the name the target still holds its contents, and with a dangling link the target is not created. A third does the same for a malformed mode, since "bw" must not empty a file on its way to being rejected, and a fourth for an exclusive mode on a name that is already there. The exclusive modes have three cases of their own: the EEXIST refusal with the file left alone, every variant C allows including the "w+x" that only works because 'x' is dropped for fdopen(), and a symlink under "wx" still refused as a link. One more asserts where an append stream starts, on both "a" and "a+". Against the previous behaviour the first fails because the target has been emptied, the second because the target now exists, and the mode and O_CLOEXEC cases fail as well. The same approach was proposed publicly and independently in #312, "pqos: safe_fopen: use O_NOFOLLOW to prevent symlink following". This implementation differs in four ways: a dangling link answers ENOENT rather than ELOOP for a "w+" open, EMLINK is what FreeBSD reports for a refused link, errno is kept for the caller, and the descriptor carries O_CLOEXEC. It also keeps the creation permissions fopen() gives a file rather than narrowing them. Co-authored-by: Tony Camuso <tcamuso@redhat.com> Signed-off-by: Raghavan Kanagaraj <raghavan.kanagaraj@intel.com>

camuso · 2026-10-15T15:50:04Z

Closing — the fixes have been incorporated into master by the maintainers. Thanks for the quick turnaround!

camuso · 2026-10-15T15:53:11Z

Thanks for incorporating the fixes into master. One note: the current implementation uses O_NOFOLLOW on the final path component, which prevents symlink following at the leaf. However, symlinks in intermediate directory components are still followed by the kernel during pathname resolution.

Our RHEL patches include a safe_openat_walk() helper that traverses each path component using openat(O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC), atomically rejecting symlinks at every level without a TOCTOU window. We also added cmocka unit tests covering this case (test_safe_fopen_rejects_intermediate_symlink).

If you are interested in the stronger intermediate-component protection, I am happy to open a follow-up PR with the openat walk and the unit tests.

rkanagar requested a balanced review from Copilot September 9, 2026 13:13

Copilot started reviewing on behalf of rkanagar September 9, 2026 13:14

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

camuso added 3 commits September 9, 2026 09:50

camuso force-pushed the fix-fd-leak-and-symlink-race branch from e558710 to c2acdc4 Compare September 14, 2026 13:43

camuso closed this Oct 15, 2026

Navigation Menu

Repository navigation

Fix privileged fd leak in libpqos and symlink race in pqos safe_fopen - #312

Fix privileged fd leak in libpqos and symlink race in pqos safe_fopen#312
camuso wants to merge 3 commits into
intel:masterfrom
camuso:fix-fd-leak-and-symlink-race

camuso commented Aug 31, 2026

Uh oh!

rkanagar commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

camuso commented Sep 14, 2026 •

edited

Loading

Uh oh!

camuso commented Oct 15, 2026

Uh oh!

camuso commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		m_apilock =
		open(LOCKFILE, O_RDWR \| O_CREAT \| O_EXCL \| O_CLOEXEC, LOCKFILE_PERMS);

Conversation

camuso commented Aug 31, 2026

Uh oh!

rkanagar commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

camuso commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camuso commented Oct 15, 2026

Uh oh!

camuso commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

camuso commented Sep 14, 2026 •

edited

Loading