[PATCH] qemu: tpm: Avoid following symlinks when chown'ing log file

Jim Fehlig via Devel posted 1 patch 6 days, 4 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260818215421.731014-1-jfehlig@suse.com
src/qemu/qemu_tpm.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
[PATCH] qemu: tpm: Avoid following symlinks when chown'ing log file
Posted by Jim Fehlig via Devel 6 days, 4 hours ago
From: Jim Fehlig <jfehlig@suse.com>

libvirt chown()s the swtpm log file to the swtpm user:group when starting
a VM. The swtpm log directory is writable by swtmp user, who could replace
the logfile with a symlink to a root-owned path. At next VM start, libvirt
will chown() that path to the swtpm user:group, which breaks the intended
separation between the confined swtpm account and root-owned files.

Use fchown() on an fd opened with O_NOFOLLOW to avoid the potential
symlink attack.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/qemu/qemu_tpm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 660410bcba..34e11cc02f 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -1030,6 +1030,8 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm,
                            uid_t qemu_user,
                            const char *shortName)
 {
+    VIR_AUTOCLOSE logfd = -1;
+
     /* create log dir ... allow 'tss' user to cd into it */
     if (g_mkdir_with_parents(logDir, 0711) < 0)
         return -1;
@@ -1039,13 +1041,20 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm,
                      VIR_DIR_CREATE_ALLOW_EXIST) < 0)
         return -1;
 
-    if (!virFileExists(tpm->data.emulator.logfile) &&
-        virFileTouch(tpm->data.emulator.logfile, 0644) < 0) {
+    /* Open (creating if necessary) the logfile without following a
+     * symlink. The log directory is writable by swtpm_user, so we want
+     * to avoid chown'ing a symlink to an arbitrary path.
+     */
+    if ((logfd = open(tpm->data.emulator.logfile,
+                      O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0644)) < 0) {
+        virReportSystemError(errno,
+                             _("Could not open swtpm logfile %1$s"),
+                             tpm->data.emulator.logfile);
         return -1;
     }
 
     /* ... and make sure it can be accessed by swtpm_user */
-    if (chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) {
+    if (fchown(logfd, swtpm_user, swtpm_group) < 0) {
         virReportSystemError(errno,
                              _("Could not chown on swtpm logfile %1$s"),
                              tpm->data.emulator.logfile);
-- 
2.51.0
Re: [PATCH] qemu: tpm: Avoid following symlinks when chown'ing log file
Posted by Martin Kletzander via Devel 5 days, 18 hours ago
On Tue, Aug 18, 2026 at 03:53:58PM -0600, Jim Fehlig via Devel wrote:
>From: Jim Fehlig <jfehlig@suse.com>
>
>libvirt chown()s the swtpm log file to the swtpm user:group when starting
>a VM. The swtpm log directory is writable by swtmp user, who could replace
>the logfile with a symlink to a root-owned path. At next VM start, libvirt
>will chown() that path to the swtpm user:group, which breaks the intended
>separation between the confined swtpm account and root-owned files.
>
>Use fchown() on an fd opened with O_NOFOLLOW to avoid the potential
>symlink attack.
>

I thought the O_NOFOLLOW was a BSDism and went to check whether it is
POSIX or not.  I looked at the man page and instantly felt old.

>Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Re: [PATCH] qemu: tpm: Avoid following symlinks when chown'ing log file
Posted by Jim Fehlig via Devel 5 days, 8 hours ago
On 8/19/26 1:38 AM, Martin Kletzander wrote:
> On Tue, Aug 18, 2026 at 03:53:58PM -0600, Jim Fehlig via Devel wrote:
>> From: Jim Fehlig <jfehlig@suse.com>
>>
>> libvirt chown()s the swtpm log file to the swtpm user:group when starting
>> a VM. The swtpm log directory is writable by swtmp user, who could replace
>> the logfile with a symlink to a root-owned path. At next VM start, libvirt
>> will chown() that path to the swtpm user:group, which breaks the intended
>> separation between the confined swtpm account and root-owned files.
>>
>> Use fchown() on an fd opened with O_NOFOLLOW to avoid the potential
>> symlink attack.
>>
> 
> I thought the O_NOFOLLOW was a BSDism and went to check whether it is
> POSIX or not.  I looked at the man page and instantly felt old.

Same. I knew Linux supported it for a while, but didn't think that many years 
have raced by :-)

"This flag is a FreeBSD extension, which was added in Linux 2.1.126, and has 
subsequently been standardized in POSIX.1-2008"

Cheers,
Jim