QProcess has the needed functionality to do this in between fork and exec
Description
Description
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Open | None | T12165 KDESu | ||
| Open | None | T12166 KDESu: Close fds manually instead of relying on KService |
Comment Actions
This could use Qt6 setupChildProcess() or in Qt5 setChildProcessModifier(), similar to https://phabricator.kde.org/T13940.
Here's how to test it:
# check that kdesud is not running yet chmod g+s $KDEDIR/lib64/libexec/kf5/kdesud QT_PLUGIN_PATH='' $KDEDIR/lib64/libexec/kf5/kdesu dolphin # check that kdesud is now running ls -l /proc/`pidof kdesu`/fd # check what this includes, e.g. the icon cache file ls -l /proc/`pidof kdesud`/fd # check that this does not include the icon cache file
Comment Actions
I changed the code in kdesu/src/client.cpp back to using system(), see https://invent.kde.org/unmaintained/kdelibs/-/commit/e4cebe4edd98d63fabfb2eecf4ceef794d5fa5a4; (I wanted to see what fd's were shared between parent and child, to test if it still works after I change the code to use QProcess):
# Built, installed in local dir, chmod g+s, 'source builddir/prefix.sh' $ QT_PLUGIN_PATH='' /usr/lib64/libexec/kf5/kdesu dolphin # Type root password in dialog, kdesud from the local install dir is used $ sudo ls -l /proc/`pidof kdesud`/fd total 0 lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 0 -> /dev/pts/2 lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 1 -> /dev/pts/2 lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 2 -> /dev/pts/2 lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 3 -> 'anon_inode:[eventfd]' lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 4 -> 'socket:[63039]' lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 5 -> 'socket:[64021]' lr-x------ 1 ahmad ahmad 64 Nov 16 11:43 6 -> 'pipe:[61979]' l-wx------ 1 ahmad ahmad 64 Nov 16 11:43 7 -> 'pipe:[61979]' lrwx------ 1 ahmad ahmad 64 Nov 16 11:43 8 -> 'socket:[61980]' $ sudo ls -l /proc/`pidof kdesu`/fd total 0 lrwx------ 1 root root 64 Nov 16 11:43 0 -> /dev/pts/2 lrwx------ 1 root root 64 Nov 16 11:43 1 -> /dev/pts/2 lrwx------ 1 root root 64 Nov 16 11:43 10 -> /dev/ptmx lrwx------ 1 root root 64 Nov 16 11:43 11 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 16 11:43 12 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 16 11:43 13 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 16 11:44 14 -> /dev/ptmx lrwx------ 1 root root 64 Nov 16 11:43 2 -> /dev/pts/2 lrwx------ 1 root root 64 Nov 16 11:43 3 -> 'socket:[64019]' lrwx------ 1 root root 64 Nov 16 11:43 4 -> 'anon_inode:[eventfd]' lrwx------ 1 root root 64 Nov 16 11:43 5 -> 'anon_inode:[eventfd]' lrwx------ 1 root root 64 Nov 16 11:43 6 -> 'socket:[61976]' lrwx------ 1 root root 64 Nov 16 11:43 7 -> 'anon_inode:[eventfd]' lrwx------ 1 root root 64 Nov 16 11:43 8 -> 'socket:[61107]' lrwx------ 1 root root 64 Nov 16 11:43 9 -> 'socket:[63040]'
I don't see the problem; maybe I am holding the widget wrong?
Reading the manual pages of fork(), it looks like we can set the process not to inherit the file descriptors of the parent process by using fcntl() on each file descriptor of the parent process, to be extra sure...
Comment Actions
Another clue:
ahmad 10822 1 0 11:43 pts/2 00:00:00 /home/ahmad/dev/kdesu/build/install-dir/lib64/libexec/kf5/kdesud
so basically, the PPID is 1, which is systemd.
Comment Actions
One more data point; using pkexec (an online search revealed that others asked the same question about pkexec before https://unix.stackexchange.com/questions/203136/how-do-i-run-gui-applications-as-root-by-using-pkexec) seems to work for e.g. yast:
pkexec /usr/sbin/yast --qt
I had to add a .policy file in /usr/share/polkit-1/actions (basically copied the gparted .policy file and adjusted it):
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE policyconfig PUBLIC
"-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
"http://www.freedesktop.org/standards/PolicyKit/1/policyconfig.dtd">
<policyconfig>
<action id="org.opensuse.policykit.yast">
<message>Authentication is required to run Yast</message>
<icon_name>yast</icon_name>
<defaults>
<allow_any>auth_admin</allow_any>
<allow_inactive>auth_admin</allow_inactive>
<allow_active>auth_admin</allow_active>
</defaults>
<annotate key="org.freedesktop.policykit.exec.path">/usr/sbin/yast</annotate>
<annotate key="org.freedesktop.policykit.exec.allow_gui">true</annotate>
</action>
</policyconfig>which raises the question, why does xdg-su not use pkexec? limitations or it doesn't cover all use cases?
Comment Actions
So, it looks like there is a viable alternative for OpenSuse's yast use-case; haven't tried the other apps that use kdesu.
Comment Actions
Using openbox:
ct 14634 14229 0 13:40 pts/8 00:00:00 kdesu /usr/bin/dolphin ct 14643 1 0 13:40 ? 00:00:00 /usr/lib64/libexec/kf5/kdesud root 14663 14634 0 13:41 pts/12 00:00:00 /usr/bin/su root -c /usr/lib64/libexec/kf5/kdesu_stub - root 14666 14663 0 13:41 pts/12 00:00:00 /usr/lib64/libexec/kf5/kdesu_stub # ls -l /proc/`pidof kdesu`/fd total 0 lrwx------ 1 root root 64 Nov 30 13:44 0 -> /dev/pts/8 lrwx------ 1 root root 64 Nov 30 13:44 1 -> /dev/pts/8 lr-x------ 1 root root 64 Nov 30 13:44 10 -> /usr/local/share/icons/hicolor/icon-theme.cache lr-x------ 1 root root 64 Nov 30 13:44 11 -> /usr/share/icons/hicolor/icon-theme.cache lrwx------ 1 root root 64 Nov 30 13:44 12 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 30 13:44 13 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 30 13:44 14 -> /dev/dri/card0 lrwx------ 1 root root 64 Nov 30 13:44 15 -> /dev/ptmx lrwx------ 1 root root 64 Nov 30 13:44 2 -> /dev/pts/8 lrwx------ 1 root root 64 Nov 30 13:44 3 -> socket:[65861] lrwx------ 1 root root 64 Nov 30 13:44 4 -> anon_inode:[eventfd] lrwx------ 1 root root 64 Nov 30 13:44 5 -> anon_inode:[eventfd] lrwx------ 1 root root 64 Nov 30 13:44 6 -> anon_inode:[eventfd] lrwx------ 1 root root 64 Nov 30 13:44 7 -> socket:[65086] lrwx------ 1 root root 64 Nov 30 13:44 8 -> socket:[64321] lrwx------ 1 root root 64 Nov 30 13:44 9 -> /dev/ptmx # ls -l /proc/`pidof kdesud`/fd total 0 lr-x------ 1 root root 64 Nov 30 13:45 0 -> pipe:[65865] lrwx------ 1 root root 64 Nov 30 13:45 1 -> /dev/pts/8 lrwx------ 1 root root 64 Nov 30 13:45 2 -> /dev/pts/8 lrwx------ 1 root root 64 Nov 30 13:45 3 -> anon_inode:[eventfd] lrwx------ 1 root root 64 Nov 30 13:45 4 -> socket:[65875] lrwx------ 1 root root 64 Nov 30 13:45 5 -> socket:[65093] lr-x------ 1 root root 64 Nov 30 13:45 6 -> pipe:[65876] l-wx------ 1 root root 64 Nov 30 13:45 7 -> pipe:[65876] lrwx------ 1 root root 64 Nov 30 13:45 8 -> socket:[65877] # ls -l /proc/`pidof kdesu_stub`/fd total 0 lrwx------ 1 root root 64 Nov 30 13:49 0 -> /dev/pts/12 lrwx------ 1 root root 64 Nov 30 13:49 1 -> /dev/pts/12 lrwx------ 1 root root 64 Nov 30 13:49 2 -> /dev/pts/12
I see the icon theme fd; still not sure if systemd is interfering or not.
Next I'll try using ::system(), then fcntl() to close the fd using *CLOEXEC.
Comment Actions
In one of the KF6 meetings, it was noted that kdesu won't work in Wayland because the latter doesn't allow GUI apps to run as root at all (IIUC).
Comment Actions
In one of the KF6 meetings, it was noted that kdesu won't work in Wayland because the latter doesn't allow GUI apps to run as root at all (IIUC).
As long as the application has access to the wayland socket, it'll work. If $WAYLAND_DISPLAY is an absolute path or $XDG_RUNTIME_DIR is preserved (kdesu doesn't do the latter), that's enough for applications running as root to connect.
e.g. sudo -E kdialog --getopenfilename or WAYLAND_DISPLAY=$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY kdesu -- kdialog --getopenfilename start kdialog with wayland as root.
Comment Actions
That's good to know (sort of more motivation for us to actually try and fix this). :)
Thanks.
Comment Actions
More measly digging:
- kdesud parent pid is 1, (so probably the parent of kdesud isn't waiting for it to finish, and it gets repartented to systemd as the init pid 1); I am not an expert on this subject
- kdesu is the parent of /usr/bin/su root -c /usr/lib64/libexec/kf5/kdesu_stub -
- the previous /usr/bin/su process is the the parent of /usr/lib64/libexec/kf5/kdesu_stub
- kdesu_stub is the parent of dolphin
- /usr/lib64/libexec/kf5/kioslave5 /usr/lib64/qt5/plugins/kf5/kio/kio_file.so has parent pid 1
the is running under OpenBox, and I get the same behaviour running the libKF5Su5 package from the distro repos, and running from a local build where I changed the code to use ::system() instead of KToolInvocation::kdeinitExecWait()
The comment in kdesu/src/client.cpp says:
// kdesud only forks to the background after it is accepting // connections. // We start it via kdeinit to make sure that it doesn't inherit // any fd's from the parent process.
Comment Actions
Debugging with:
diff --git a/src/kdesud/kdesud.cpp b/src/kdesud/kdesud.cpp index e9b52bf..fe5d478 100644 --- a/src/kdesud/kdesud.cpp +++ b/src/kdesud/kdesud.cpp @@ -288,6 +288,7 @@ int main(int argc, char *argv[]) if (sockfd < 0) { exit(1); } + if (listen(sockfd, 10) < 0) { qCCritical(KSUD_LOG) << "listen(): " << ERR << "\n"; kdesud_cleanup(); @@ -295,8 +296,11 @@ int main(int argc, char *argv[]) } int maxfd = sockfd; + qDebug() << "Before fork(): process id" << getpid() << "parent process id" << getppid(); // Ok, we're accepting connections. Fork to the background. pid_t pid = fork(); + qDebug() << "process id" << getpid() << "parent process id" << getppid(); + if (pid == -1) { qCCritical(KSUD_LOG) << "fork():" << ERR << "\n"; kdesud_cleanup();
I see:
Before fork(): process id 26263 ppid 24447
process id 26263 parent process id 24447
process id 26264 parent process id 26263
the parent process 26263 is gone by the time the dialog pops up (kdesud is 26264). This is the same behaviour with the current code in the repo, and with changing to system() (in KDEsuClient::startServer()):
- int ret = KToolInvocation::kdeinitExecWait(d->daemon);
+ int ret = ::system(d->daemon.toUtf8().constData());
so far the parent of the app called with kdesu app is kdesu_stub.
Comment Actions
Using ::system() and setting FD_CLOEXEC on all fd's, I still get similar results.
diff --git a/src/client.cpp b/src/client.cpp index 29f5f8f..424face 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -430,7 +430,9 @@ int KDEsuClient::startServer() // connections. // We start it via kdeinit to make sure that it doesn't inherit // any fd's from the parent process. - int ret = KToolInvocation::kdeinitExecWait(d->daemon); +// int ret = KToolInvocation::kdeinitExecWait(d->daemon); + int ret = ::system(d->daemon.toUtf8().constData()); + connect(); return ret; } diff --git a/src/kdesud/kdesud.cpp b/src/kdesud/kdesud.cpp index e9b52bf..f27025f 100644 --- a/src/kdesud/kdesud.cpp +++ b/src/kdesud/kdesud.cpp @@ -158,8 +158,6 @@ void sigchld_handler(int) int create_socket() { - int sockfd; - socklen_t addrlen; struct stat s; QString display = QString::fromLocal8Bit(qgetenv("DISPLAY")); @@ -195,6 +193,9 @@ int create_socket() } } + int sockfd; + socklen_t addrlen; + sockfd = socket(PF_UNIX, SOCK_STREAM, 0); if (sockfd < 0) { qCCritical(KSUD_LOG) << "socket(): " << ERR << "\n"; @@ -220,7 +221,8 @@ int create_socket() _fd = -1; } int _fd; - } guard(sockfd); + }; + fd_ScopeGuard guard(sockfd); struct sockaddr_un addr; addr.sun_family = AF_UNIX; @@ -288,6 +290,7 @@ int main(int argc, char *argv[]) if (sockfd < 0) { exit(1); } + if (listen(sockfd, 10) < 0) { qCCritical(KSUD_LOG) << "listen(): " << ERR << "\n"; kdesud_cleanup(); @@ -295,8 +298,11 @@ int main(int argc, char *argv[]) } int maxfd = sockfd; + fcntl(sockfd, F_SETFD, FD_CLOEXEC); + // Ok, we're accepting connections. Fork to the background. pid_t pid = fork(); + if (pid == -1) { qCCritical(KSUD_LOG) << "fork():" << ERR << "\n"; kdesud_cleanup(); @@ -309,6 +315,9 @@ int main(int argc, char *argv[]) #if HAVE_X11 // Make sure we exit when the display gets closed. int x11Fd = initXconnection(); + + fcntl(x11Fd, F_SETFD, FD_CLOEXEC); + maxfd = qMax(maxfd, x11Fd); #endif @@ -316,6 +325,9 @@ int main(int argc, char *argv[]) QVector<ConnectionHandler *> handler; pipe(pipeOfDeath); + fcntl(pipeOfDeath[0], F_SETFD, FD_CLOEXEC); + fcntl(pipeOfDeath[1], F_SETFD, FD_CLOEXEC); + maxfd = qMax(maxfd, pipeOfDeath[0]); // Signal handlers @@ -414,6 +426,9 @@ int main(int argc, char *argv[]) qCCritical(KSUD_LOG) << "accept():" << ERR << "\n"; continue; } + + fcntl(fd, F_SETFD, FD_CLOEXEC); + while (fd + 1 > (int)handler.size()) { handler.append(nullptr); }
Comment Actions
Finally merged https://invent.kde.org/frameworks/kdesu/-/merge_requests/14
Thanks to @fvogt for the big help through it all :)