RFC: automatic _NET_WM_PID setting for local clients

Submitted by xorg@pengaru.com on July 23, 2019, 8:57 p.m.

Details

Message ID 20190723205701.mzwjz5ypivtz2xby@shells.gnugeneration.com
State New
Headers show
Series "RFC: automatic _NET_WM_PID setting for local clients" ( rev: 1 ) in X.org

Not browsing as part of any series.

Commit Message

xorg@pengaru.com July 23, 2019, 8:57 p.m.
Hello folks,

I'd like to propose that Xorg set the _NET_WM_PID property on new
windows for local clients @ window create time whenever possible.  

This is something I added localy years ago to more reliably have this
property set with uncooperative clients that didn't set it.  My window
manager integrates client process monitoring and relies on this property
for acquiring the PID of connected clients.

At the time, it was just a few X clients that were problematic, stuff
like xpdf and other smaller programs using less popular toolkits or no
toolkit at all.  It wasn't such a big deal, so I promptly forgot about
it and stopped building my own Xorg debs with the patch, living with the
absent monitoring overlays on those windows.

Fast-forward to today; I'm using systemd-nspawn for running X clients -
particularly network-facing clients like FireFox where I _strongly_
prefer isolating the client from accessing things like my home directory
for obvious reasons (ssh keys, etc).

These programs are cooperative and set _NET_WM_PID, but the PID they set
is from the perspective of the container namespace.  The display server
is running in the global host namespace, where this PID has zero
relevance.  The same goes for my window manager, it to runs in the
host's namespace, so when it gets this PID and tries to monitor the
process subtree rooted at that PID in /proc, it either finds nothing or
the entirely wrong tree.

So again I'm wishing the display server would just set this property for
local clients immediately when creating the window, which would not only
make the property more reliable but now it would also set it from the
PIDNS of the Xorg server, that I would argue is far more meaningful.

I happened to still have the old patch I was using to do this back in
the day, and have attached it as-is for discussion purposes.

Thanks,
Vito Caputo

Patch hide | download patch | download mbox

--- xorg-server-1.12.4/dix/window.c	2012-05-17 10:09:02.000000000 -0700
+++ xorg-server-1.12.4.hacked/dix/window.c	2014-06-04 18:54:33.570855708 -0700
@@ -840,6 +840,20 @@ 
         event.u.createNotify.override = pWin->overrideRedirect;
         DeliverEvents(pParent, &event, 1, NullWindow);
     }
+
+    if (pScreen->root == pParent) {
+        /* top-level windows with local connections can reliably get _NET_WM_PID set by the server */
+        LocalClientCredRec *lcc;
+        if (GetLocalClientCreds(client, &lcc) != -1)
+            if (lcc->fieldsSet & LCC_PID_SET) {
+                Atom prop;
+
+                prop = MakeAtom("_NET_WM_PID", strlen("_NET_WM_PID"), TRUE);
+                dixChangeWindowProperty(client, pWin, prop,
+                                        XA_CARDINAL, 32, PropModeReplace,
+                                        1, &lcc->pid, FALSE);
+            }
+    }
     return pWin;
 }
 

Comments

On Tue, 23 Jul 2019 13:57:01 -0700 xorg@pengaru.com said:

it'd be much more reliable to set _NET_STARTUP_ID to the content of whatever
the DESKTOP_STARTUP_ID env var has and enforce this in xlib itself. this can be
inherited down the chain through your launching/containers/whatever and passed
in for that launch instance. assuming your wm of course can do this and track
every launch instance it started off and map it back to that instance... but it
can know reliably then "THIS action of launching here resulted in that window
over there". much better than _NET_WM_PID because the pid here may not be the
pid of whatever was forked - but some other child process or even unrelated
other pid.

> Hello folks,
> 
> I'd like to propose that Xorg set the _NET_WM_PID property on new
> windows for local clients @ window create time whenever possible.  
> 
> This is something I added localy years ago to more reliably have this
> property set with uncooperative clients that didn't set it.  My window
> manager integrates client process monitoring and relies on this property
> for acquiring the PID of connected clients.
> 
> At the time, it was just a few X clients that were problematic, stuff
> like xpdf and other smaller programs using less popular toolkits or no
> toolkit at all.  It wasn't such a big deal, so I promptly forgot about
> it and stopped building my own Xorg debs with the patch, living with the
> absent monitoring overlays on those windows.
> 
> Fast-forward to today; I'm using systemd-nspawn for running X clients -
> particularly network-facing clients like FireFox where I _strongly_
> prefer isolating the client from accessing things like my home directory
> for obvious reasons (ssh keys, etc).
> 
> These programs are cooperative and set _NET_WM_PID, but the PID they set
> is from the perspective of the container namespace.  The display server
> is running in the global host namespace, where this PID has zero
> relevance.  The same goes for my window manager, it to runs in the
> host's namespace, so when it gets this PID and tries to monitor the
> process subtree rooted at that PID in /proc, it either finds nothing or
> the entirely wrong tree.
> 
> So again I'm wishing the display server would just set this property for
> local clients immediately when creating the window, which would not only
> make the property more reliable but now it would also set it from the
> PIDNS of the Xorg server, that I would argue is far more meaningful.
> 
> I happened to still have the old patch I was using to do this back in
> the day, and have attached it as-is for discussion purposes.
> 
> Thanks,
> Vito Caputo
On Wed, Jul 24, 2019 at 01:34:37AM +0100, Carsten Haitzler wrote:
> On Tue, 23 Jul 2019 13:57:01 -0700 xorg@pengaru.com said:
> > Hello folks,
> > 
> > I'd like to propose that Xorg set the _NET_WM_PID property on new
> > windows for local clients @ window create time whenever possible.  
> > 
> > This is something I added localy years ago to more reliably have this
> > property set with uncooperative clients that didn't set it.  My window
> > manager integrates client process monitoring and relies on this property
> > for acquiring the PID of connected clients.
> > 
> > At the time, it was just a few X clients that were problematic, stuff
> > like xpdf and other smaller programs using less popular toolkits or no
> > toolkit at all.  It wasn't such a big deal, so I promptly forgot about
> > it and stopped building my own Xorg debs with the patch, living with the
> > absent monitoring overlays on those windows.
> > 
> > Fast-forward to today; I'm using systemd-nspawn for running X clients -
> > particularly network-facing clients like FireFox where I _strongly_
> > prefer isolating the client from accessing things like my home directory
> > for obvious reasons (ssh keys, etc).
> > 
> > These programs are cooperative and set _NET_WM_PID, but the PID they set
> > is from the perspective of the container namespace.  The display server
> > is running in the global host namespace, where this PID has zero
> > relevance.  The same goes for my window manager, it to runs in the
> > host's namespace, so when it gets this PID and tries to monitor the
> > process subtree rooted at that PID in /proc, it either finds nothing or
> > the entirely wrong tree.
> > 
> > So again I'm wishing the display server would just set this property for
> > local clients immediately when creating the window, which would not only
> > make the property more reliable but now it would also set it from the
> > PIDNS of the Xorg server, that I would argue is far more meaningful.
> > 
> > I happened to still have the old patch I was using to do this back in
> > the day, and have attached it as-is for discussion purposes.
> > 
> > Thanks,
> > Vito Caputo
> 
> it'd be much more reliable to set _NET_STARTUP_ID to the content of whatever
> the DESKTOP_STARTUP_ID env var has and enforce this in xlib itself. this can be
> inherited down the chain through your launching/containers/whatever and passed
> in for that launch instance. assuming your wm of course can do this and track
> every launch instance it started off and map it back to that instance... but it
> can know reliably then "THIS action of launching here resulted in that window
> over there". much better than _NET_WM_PID because the pid here may not be the
> pid of whatever was forked - but some other child process or even unrelated
> other pid.
> 
> -- 
> ------------- Codito, ergo sum - "I code, therefore I am" --------------
> Carsten Haitzler - raster@rasterman.com

These aren't mutually exclusive proposals and in my opinion are somewhat
complementary.

Consider that in my workflow I am often launching X clients manually
from bash via xterm.  These won't have new startup IDs allocated to
them, so the monitoring overlays would reflect those of the top-level
WM-launched xterm.  If the client PID matches somewhere deeper in the
heirarchy rooted by the startup ID, I'd default to narrowing the scope
to that level.  It's /probably/ going to be the right thing, so I'd
still want the X client PID.

Speaking from years of experience using this scheme, keying off the PID
alone has been perfectly functional until namespaces came along.  Maybe
it should be on a different property, like _UNIX_WM_PID or something,
just to not step on the existing sematics of _NET_WM_PID.  But I think
it's perfectly reasonable to have the X server make this client
identifier as viewed by the X server more reliably accessible when
possible, considering it's the X server with the AF_UNIX socket and
associated creds at its disposal.

Thanks,
Vito Caputo