From 00a2fd6f22a0067d215e5ebf2fb6f26b4f08e1cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hans=20J=C3=B8rgen=20Hoel?= <hansjorg@urkilden.no>
Date: Sat, 13 Jun 2026 02:50:21 +0200
Subject: [PATCH] virtio-fs: restore the original thread credentials, not
 euid/egid 0
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

scoped_cred's Drop resets the worker thread's effective uid/gid to a
hardcoded 0, assuming the server runs as root. That breaks an
unprivileged server granted CAP_SETUID/CAP_SETGID (e.g. via systemd's
AmbientCapabilities=): set_creds sees the capability and switches, but
restoring to 0 poisons the thread. A first switched request's Drop parks
the thread at euid 0; the next switch to a non-zero uid clears the
thread's effective capabilities (capabilities(7), "Effect of user ID
changes on capabilities"); that request's Drop then fails EPERM, and the
thread is stuck with the guest uid's credentials. Every later request it
handles -- including ones for guest root -- fails EACCES/EPERM on host
files that uid cannot reach.

Any guest issuing requests as more than one uid wedges within a few ops.
A Debian guest running 'apt-get update' (whose fetch methods drop to the
_apt user) reproduces it: the InRelease/Packages renames fail with
'rename failed, Permission denied'.

Capture the effective id before switching and restore that on drop. A
root-run server captures 0 and is unchanged; an unprivileged server now
only switches between non-zero uids, never clearing its caps -- and the
worker thread no longer escalates itself to host euid 0.

Signed-off-by: Hans Jørgen Hoel <hansjorg@urkilden.no>
Assisted-by: Claude Code: claude-fable-5
(cherry picked from commit 4c9c185c8ac02b1bb21420834b5651767f0c9cc9)
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 .../src/virtio/fs/linux/passthrough.rs        | 30 ++++++++++++++-----
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/devices/src/virtio/fs/linux/passthrough.rs b/src/devices/src/virtio/fs/linux/passthrough.rs
index f85faef..5463f50 100644
--- a/src/devices/src/virtio/fs/linux/passthrough.rs
+++ b/src/devices/src/virtio/fs/linux/passthrough.rs
@@ -69,13 +69,16 @@ struct LinuxDirent64 {
 unsafe impl ByteValued for LinuxDirent64 {}
 
 macro_rules! scoped_cred {
-    ($name:ident, $ty:ty, $syscall_nr:expr) => {
+    ($name:ident, $ty:ty, $syscall_nr:expr_2021, $get_current:expr_2021) => {
         #[derive(Debug)]
-        struct $name;
+        struct $name {
+            old: $ty,
+        }
 
         impl $name {
             // Changes the effective uid/gid of the current thread to `val`.  Changes
-            // the thread's credentials back to root when the returned struct is dropped.
+            // the thread's credentials back to the previous value when the returned
+            // struct is dropped.
             fn new(val: $ty) -> io::Result<Option<$name>> {
                 // We want credential changes to be per-thread because otherwise
                 // we might interfere with operations being carried out on other
@@ -89,11 +92,22 @@ macro_rules! scoped_cred {
                 // setfsgid systems calls.   However since those calls have no way to
                 // return an error, it's preferable to do this instead.
 
+                // Remember the current effective id so Drop can restore it.
+                // Restoring a hardcoded 0 instead is wrong when the server
+                // runs as an unprivileged user granted CAP_SETUID/CAP_SETGID:
+                // the first Drop parks the thread at euid 0, the next switch
+                // to a non-zero uid then clears the thread's effective
+                // capability set (see capabilities(7), "Effect of user ID
+                // changes on capabilities"), and the restore after that fails
+                // with EPERM -- leaving the worker thread stuck with the guest
+                // uid's credentials for every subsequent request.
+                let old = unsafe { $get_current() } as $ty;
+
                 // This call is safe because it doesn't modify any memory and we
                 // check the return value.
                 let res = unsafe { libc::syscall($syscall_nr, -1, val, -1) };
                 if res == 0 {
-                    Ok(Some($name))
+                    Ok(Some($name { old }))
                 } else {
                     Err(io::Error::last_os_error())
                 }
@@ -102,10 +116,10 @@ macro_rules! scoped_cred {
 
         impl Drop for $name {
             fn drop(&mut self) {
-                let res = unsafe { libc::syscall($syscall_nr, -1, 0, -1) };
+                let res = unsafe { libc::syscall($syscall_nr, -1, self.old, -1) };
                 if res < 0 {
                     error!(
-                        "failed to change credentials back to root: {}",
+                        "failed to restore credentials: {}",
                         io::Error::last_os_error(),
                     );
                 }
@@ -113,8 +127,8 @@ macro_rules! scoped_cred {
         }
     };
 }
-scoped_cred!(ScopedUid, libc::uid_t, libc::SYS_setresuid);
-scoped_cred!(ScopedGid, libc::gid_t, libc::SYS_setresgid);
+scoped_cred!(ScopedUid, libc::uid_t, libc::SYS_setresuid, libc::geteuid);
+scoped_cred!(ScopedGid, libc::gid_t, libc::SYS_setresgid, libc::getegid);
 
 #[must_use]
 pub struct ScopedCaps {
-- 
2.51.2