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
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>
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
© 2016 - 2026 Red Hat, Inc.