[PATCH RFC] Xorg: Add a suid root wrapper
With the recent systemd-logind changes it is possible to install the Xorg
binary without suid root rights and still have everything working as it
should *if* the user only has cards which are supported by kms.
This commit adds a little suid root wrapper, which is a bit weird, first we
strip the suid-root bit of the Xorg binary, and then we add a wrapper ?
The function of this wrapper is to see if a system still needs root-rights,
if it does not (it supports kms and the kms drivers are properly loaded),
then it will immediately drop all elevated rights before executing the real
Xorg binary. If it finds (some) cards which don't support kms, or no cards
at all, then it will execute the Xorg server with elevated rights so that
ie the nvidia binary driver and the vesa driver can keep working normally.
To make it possible for security concious users who don't need the root
rights to completely remove the wrapper, Xorg is started in a 3 step process
when the wrapper is enabled during build time:
1) A simple shell script which checks if the wrapper is there, if it is
it executes the wrapper, if not it directly executes the real Xorg binary
2) The wrapper gets executed, does its checks, normally drops all elevated
rights and then executes the real Xorg binary
3) The real Xorg binary does its thing
This allows distributions to put the wrapper binary in a separate package, and
will allow users to remove this package. IE the plan with Fedora is to make
legacy drivers depend on the wrapper pkg, and since our default install
contains some legacy drivers it will be part of the default install, but
users can later yum remove it (which will also automatically remove the
legacy driver packages as those won't work without it anyways).
Signed-off-by: Hans de Goede hdego...@redhat.com
---
configure.ac | 6
hw/xfree86/Makefile.am| 12 ++-
hw/xfree86/Xorg.sh| 13
hw/xfree86/xorg-wrapper.c | 82 +++
4 files changed, 112 insertions(+), 1 deletion(-)
create mode 100644 hw/xfree86/Xorg.sh
create mode 100644 hw/xfree86/xorg-wrapper.c
diff --git a/configure.ac b/configure.ac
index 588ae59..5e96911 100644
--- a/configure.ac
+++ b/configure.ac
@@ -627,6 +627,7 @@ AC_ARG_ENABLE(pciaccess,
AS_HELP_STRING([--enable-pciaccess], [Build Xorg with p
AC_ARG_ENABLE(linux_acpi, AS_HELP_STRING([--disable-linux-acpi], [Disable
building ACPI support on Linux (if available).]),
[enable_linux_acpi=$enableval], [enable_linux_acpi=yes])
AC_ARG_ENABLE(linux_apm, AS_HELP_STRING([--disable-linux-apm], [Disable
building APM support on Linux (if available).]), [enable_linux_apm=$enableval],
[enable_linux_apm=yes])
AC_ARG_ENABLE(systemd-logind, AC_HELP_STRING([--enable-systemd-logind], [Build
systemd-logind support (default: auto)]), [SYSTEMD_LOGIND=$enableval],
[SYSTEMD_LOGIND=auto])
+AC_ARG_ENABLE(suid-wrapper, AC_HELP_STRING([--enable-suid-wrapper], [Build
suid-root wrapper for legacy driver support on rootless xserver systems
(default: no)]), [SUID_WRAPPER=$enableval], [SUID_WRAPPER=no])
dnl DDXes.
AC_ARG_ENABLE(xorg, AS_HELP_STRING([--enable-xorg], [Build
Xorg server (default: auto)]), [XORG=$enableval], [XORG=auto])
@@ -922,6 +923,11 @@ if test x$SYSTEMD_LOGIND = xyes; then
fi
AM_CONDITIONAL(SYSTEMD_LOGIND, [test x$SYSTEMD_LOGIND = xyes])
+if test x$SUID_WRAPPER = xyes; then
+SETUID=no
+fi
+AM_CONDITIONAL(SUID_WRAPPER, [test x$SUID_WRAPPER = xyes])
+
if test x$NEED_DBUS = xyes; then
AC_DEFINE(NEED_DBUS, 1, [Enable D-Bus core])
fi
diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am
index 9672904..d44d172 100644
--- a/hw/xfree86/Makefile.am
+++ b/hw/xfree86/Makefile.am
@@ -76,9 +76,14 @@ Xorg_DEPENDENCIES = $(LOCAL_LIBS)
Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG)
+if SUID_WRAPPER
+bin_PROGRAMS += Xorg.wrap
+Xorg_wrap_SOURCES = xorg-wrapper.c
+endif
+
BUILT_SOURCES = xorg.conf.example
DISTCLEANFILES = xorg.conf.example
-EXTRA_DIST = xorgconf.cpp
+EXTRA_DIST = xorgconf.cpp Xorg.sh
# Without logdir, X will post an error on the terminal and will not start
install-data-local:
@@ -93,6 +98,11 @@ if INSTALL_SETUID
chown root $(DESTDIR)$(bindir)/Xorg
chmod u+s $(DESTDIR)$(bindir)/Xorg
endif
+if SUID_WRAPPER
+ mv $(DESTDIR)$(bindir)/Xorg $(DESTDIR)$(bindir)/Xorg.bin
+ ${INSTALL} -m 755 Xorg.sh $(DESTDIR)$(bindir)/Xorg
+ -chown root $(DESTDIR)$(bindir)/Xorg.wrap chmod u+s
$(DESTDIR)$(bindir)/Xorg.wrap
+endif
uninstall-local:
if CYGWIN
diff --git a/hw/xfree86/Xorg.sh b/hw/xfree86/Xorg.sh
new file mode 100644
index 000..3a2b34b
--- /dev/null
+++ b/hw/xfree86/Xorg.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# See if Xorg.wrap is installed and if it is execute it, otherwise execute
+# Xorg.bin directly. This allows distros to put the suid wrapper in a
+# separate package.
+
+canonical=$(readlink -f $0)
+basedir=$(dirname $canonical)
+if [ -x
[PATCH RFC 0/1] Xorg: Add a suid root wrapper
Hi All, Let me repeat the commit msg here as that explains it all: With the recent systemd-logind changes it is possible to install the Xorg binary without suid root rights and still have everything working as it should *if* the user only has cards which are supported by kms. This commit adds a little suid root wrapper, which is a bit weird, first we strip the suid-root bit of the Xorg binary, and then we add a wrapper ? The function of this wrapper is to see if a system still needs root-rights, if it does not (it supports kms and the kms drivers are properly loaded), then it will immediately drop all elevated rights before executing the real Xorg binary. If it finds (some) cards which don't support kms, or no cards at all, then it will execute the Xorg server with elevated rights so that ie the nvidia binary driver and the vesa driver can keep working normally. To make it possible for security concious users who don't need the root rights to completely remove the wrapper, Xorg is started in a 3 step process when the wrapper is enabled during build time: 1) A simple shell script which checks if the wrapper is there, if it is it executes the wrapper, if not it directly executes the real Xorg binary 2) The wrapper gets executed, does its checks, normally drops all elevated rights and then executes the real Xorg binary 3) The real Xorg binary does its thing This allows distributions to put the wrapper binary in a separate package, and will allow users to remove this package. IE the plan with Fedora is to make legacy drivers depend on the wrapper pkg, and since our default install contains some legacy drivers it will be part of the default install, but users can later yum remove it (which will also automatically remove the legacy driver packages as those won't work without it anyways). Note currently this patch is only RFC as I still need to test it on a system which actually needs it to keep the root rights. Still I expect there will be some discussion / review comments so I thought it would be good to post this now. Please review. Thanks Regards, Hans ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Wrong virtualX when using vesa driver with no shadowFB
On Fri, 2014-02-14 at 11:47 +, Stefano Panella wrote: The reason why the problem happen is that in the vesa driver, as it is now, has no way to detect the needed alignment of the framebuffer unless using the shadowFB. I'd say more like doesn't than has no way to. The VBE (as I understand) has only two ways to say the driver what the stride should be for a given mode: 1) mode-BytesPerScanline 2) VBEGetLogicalScanline() There's also mode-XCharSize, which in the example you give is 8; which would explain things, since 1366 % 8 != 0. But BytesPerScanline is probably a more obvious thing to look at. The second information is never used since VESASetMode() is only using VBESetLogicalScanline() and is never checking if the value was accepted from the HW or was rounded up and it looks like for mode 1366x768 BytesPerScanline=5504, pScrn-displayWidth if 1368, which results in a scanline of 5472, which will result in a distorted image. if (data-data-XResolution != pScrn-displayWidth) VBESetLogicalScanline(pVesa-pVbe, pScrn-displayWidth); The spec say that the HW could round up the value written with VBESetLogicalScanline() (and our emulation is doing so) so I think it would be appropriate to call VBEGetLogicalScanline() after to check if the value was accepted as it was or if it was rounded up, in which case pScrn-displayWidth and pScrn-virtualX should be updated accordingly. If my understanding is correct, this would explain why when we disable shadowFB the actual stride is wrong. If you can confirm my reasoning is correct, I would be happy to post a patch to fix this in case shadowFB is not used. I think your understanding is correct. Sorry for being slow to respond to this, and thanks for investigating it! - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libxtrans] configure: under glibc define _GNU_SOURCE rather then _BSD_SOURCE
On 03/ 4/14 11:18 PM, Hans de Goede wrote: Beginner question: Are the more consumers of xtrans then just the xserver ? Yes - we wouldn't have it as a separate module if there weren't. % grep -il xtrans */*/configure.ac app/lbxproxy/configure.ac app/proxymngr/configure.ac app/xauth/configure.ac app/xdm/configure.ac app/xfs/configure.ac app/xfwp/configure.ac app/xhost/configure.ac app/xrx/configure.ac app/xscope/configure.ac lib/libFS/configure.ac lib/libICE/configure.ac lib/libSM/configure.ac lib/libX11/configure.ac lib/libXfont/configure.ac lib/libXmu/configure.ac lib/libxtrans/configure.ac test/xhiv/configure.ac test/xts/configure.ac Of course, lbxproxy is deprecated now, and we should probably say the same about proxymngr, xfwp, and xrx; but we certainly can't say that about libX11, xauth, xhost, libICE, or libSM. (Some may debate xfs/libFS or xdm, but there's enough users of those to keep them going a while longer.) -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/5] miinitext: avoid allocating a sentinel ExtensionModule
On Mon, 2014-02-24 at 22:17 +, Emil Velikov wrote: Hello gents, Can someone take a look at this patch please. Keith already covered the rest of the series but this patch went below his radar. Reviewed-by: Adam Jackson a...@redhat.com - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: glamor: Enabling xephyr and Xorg.
On Mon, 2014-02-24 at 17:47 -0800, Eric Anholt wrote: Here's a short series to make the server glamor stuff actually testable. I'm now running glamor for my normal desktop, and the xephyr bits work too. I pulled out changes that would have required more from the intel 2d driver: now it's just a matter of not linking to the external module's libglamor.so, and not trying to do its own DRI3 support. I also pulled out the swap events code: While my timing tracecs on swap events are looking reasonable, there's really weird stuff going on with x11perf performance both before and after that change, and I'm holding off until I can understand it. Series (v3 of 4/6 and v2 of 5/6) is: Reviewed-by: Adam Jackson a...@redhat.com - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Switching between a software and a hardware mouse cursor
On Thu, 2014-02-27 at 15:46 +0100, Michael Thayer wrote: Another would be to add a return value to the DDX CRTC functions load_cursor_argb, so that if the KMS driver failed to set the cursor, modesetting could pass this on to the X server. Actually we really should make load_cursor_argb return something other than void. Some particularly incapable server GPUs have format restrictions on their hardware cursors that mean _some_ ARGB cursors could be made to work but not all; and right now, the fallback code in -modesetting doesn't get this case right, and you end up with no cursor at all. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Detecting whether the X server VT is the active one
On Thu, 2014-02-27 at 16:01 +0100, Michael Thayer wrote: Hello, Another problem that I am running into is that the X11 client part of our Guest Additions needs to know whether or not the X server VT is currently the active one, for example so that it can disable our seamless windows functionality if the server is switched out. Currently we do this too with a hack in our DDX. This only needs to work with a standard X.Org server, so I thought about adding code to the server to add and update a root window property to pass on the information. Does that sound like an acceptable solution, and if not, does anyone have a better one? I think that sounds reasonable. There's already cases of people getting confused about things like XGetImage not working when VT switched away. If we're going to preserve that behaviour - which I do wish would eventually be optional - it's reasonable to give clients a way to know that it's happening in-band with the rest of X. (Consulting systemd or consolekit or whatever isn't generally useful, since if the client is on another machine there's no way it could talk to the appropriate daemon.) - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Communicating with the x-server
On Tue, 2014-03-04 at 12:45 +0100, Thomas Sondergaard wrote: My own thought was simply to have the x-server watch a property, say OPEN_URI, on the root window. Would that be the way to go? Well, that or add another request to the VNC extension. I'm not sure if there's a private namespace there (to avoid your request colliding with someone else's VNC feature addition). But if there is then that's probably what I'd do. The realvnc-based code base I'm working with doesn't have any code that watches properties like this. Is there a way to snoop on PropertyNotify events? Inside the server? Not really, no. The best you could do would be to wrap the slot for ProcChangeProperty in the dispatch vector to notice success and call out to your vnc code if the property matches. Which is ugly, but avoids needing to modify the core server. If you're okay with modifying the server it'd be cleaner to add a CallCallbacks() just before the return Success at the end of dixChangeWindowProperty. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Communicating with the x-server
Hi Adam, On 2014-03-05 20:10, Adam Jackson wrote: On Tue, 2014-03-04 at 12:45 +0100, Thomas Sondergaard wrote: My own thought was simply to have the x-server watch a property, say OPEN_URI, on the root window. Would that be the way to go? Well, that or add another request to the VNC extension. I'm not sure if there's a private namespace there (to avoid your request colliding with someone else's VNC feature addition). But if there is then that's probably what I'd do. That is exactly what I did. Turned out to be very straightforward. I simply overlooked this obvious possibility. The realvnc-based code base I'm working with doesn't have any code that watches properties like this. Is there a way to snoop on PropertyNotify events? Inside the server? Not really, no. The best you could do would be to wrap the slot for ProcChangeProperty in the dispatch vector to notice success and call out to your vnc code if the property matches. Which is ugly, but avoids needing to modify the core server. If you're okay with modifying the server it'd be cleaner to add a CallCallbacks() just before the return Success at the end of dixChangeWindowProperty. Ok. Thanks a lot. Thomas ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PULL] glamor xephyr and xorg changes
The following changes since commit b634e909895f6001e7d9543e1350b20c82c8c01c: hw/xwin: More closely follow ICCCM for setting input focus (2014-03-03 14:33:09 +) are available in the git repository at: git://people.freedesktop.org/~anholt/xserver glamor-pull-request for you to fetch changes up to da08316605b26830b4d8f8fb2d9e69471cdc80ab: glamor: Add support for DRI3. (2014-03-05 13:10:24 -0800) Eric Anholt (6): xephyr: Build support for rendering with glamor using a -glamor option. xephyr: Pass incoming XCB events to the Xlib event filter. xorg: Build a glamor_egl module. xorg: Connect up the glamor XV code, xorg DDX-only for now. glamor: Rename the DRI-related pixmap functions. glamor: Add support for DRI3. configure.ac | 14 ++ glamor/Makefile.am | 1 - glamor/glamor.c| 21 ++- glamor/glamor.h| 64 --- glamor/glamor_egl.c| 97 ++- glamor/glamor_eglmodule.c | 3 +- glamor/glamor_xv.c | 19 +-- hw/kdrive/ephyr/Makefile.am| 20 ++- hw/kdrive/ephyr/ephyr.c| 45 +++-- hw/kdrive/ephyr/ephyr.h| 14 ++ hw/kdrive/ephyr/ephyr_glamor_glx.c | 331 + hw/kdrive/ephyr/ephyr_glamor_glx.h | 73 hw/kdrive/ephyr/ephyrinit.c| 14 ++ hw/kdrive/ephyr/hostx.c| 126 +- hw/xfree86/Makefile.am | 7 +- hw/xfree86/glamor_egl/Makefile.am | 43 + include/dix-config.h.in| 3 + 17 files changed, 827 insertions(+), 68 deletions(-) create mode 100644 hw/kdrive/ephyr/ephyr_glamor_glx.c create mode 100644 hw/kdrive/ephyr/ephyr_glamor_glx.h create mode 100644 hw/xfree86/glamor_egl/Makefile.am pgpHAeLZZLXwW.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH RFC] Xorg: Add a suid root wrapper
On 03/ 5/14 07:51 AM, Hans de Goede wrote: This commit adds a little suid root wrapper, which is a bit weird, first we strip the suid-root bit of the Xorg binary, and then we add a wrapper ? Have you looked at Debian's Xwrapper and considered adopting it, or at least some of its functionality? (The overall plan seems reasonable, but I think they've already thought through some details you're currently missing.) -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - http://blogs.oracle.com/alanc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
