Re: [PATCH RFC] Xorg: Add a suid root wrapper
Hi, On 03/06/2014 06:40 AM, Alan Coopersmith wrote: 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? I was not aware of Debian's Xwrapper, thanks for pointing it out to me. (The overall plan seems reasonable, but I think they've already thought through some details you're currently missing.) So looking at Debian's Xwrapper I see several things in there: 1) It has a config file to determine which users are allowed to run the xserver through the wrapper, with 3 possible settings: a) root only b) users on the local console only c) everyone Having a config file seems like a good idea, I was a bit worried about my wrapper thinking it would be ok to drop root rights when it would not be on hybrid gpu laptop configs. So it would be good to have an override option for this in a config file 2) If configured to do so it will only allow local console users to start X through the wrapper, this to seems a useful thing to have 3) It installs the wrapper as /usr/bin/X rather then /usr/bin/Xorg, and keeps the real server as /usr/bin/Xorg I think this is a bad idea since some users (ie gdm) directly execute /usr/bin/Xorg 4) It checks to see if the real xserver it will execute is not a symlink to the wrapper, and it does so in a rather naive way (using hardcoded paths for the possible wrapper install location) since both the wrapper and the Xserver should be in directories which can only be written by root, and an absolute path is used I don't see this as a necessary thing to do. If an attacker can replace the real Xserver binary with a symlink to the wrapper, he can do much worse things then replace it with a symlink back to the wrapper. This is likely to check for some configuration error caused by 3, all in all I don't see this as useful. 5) It does a check if the real Xserver is executable by the real uid, this is mostly yet another broken config check but it cannot hurt, so I'll add this to my wrapper too. 6) It does some checks on /tmp/.X11, if we want these we should do them in the server itself and not in the wrapper IMHO. Unfortunately the Xwrapper code is GPL, so not suitable for us, still there are some good ideas in there, so I'll extend my wrapper to use them. I'll keep the config-file location and format compatible so that hopefully Debian can simply switch over to my wrapper in the future. So TL;DR: Yes there are some good ideas in there, I'll add those to the next revision of my wrapper. 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: [PATCH RFC 0/1] Xorg: Add a suid root wrapper
From: Hans de Goede hdego...@redhat.com Date: Wed, 5 Mar 2014 16:51:51 +0100 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. Oh dear, the wrapper script is back! Before you go further down this road, may I point out the privilege seperation support that we've had in xenocara (Xorg for OpenBSD) for years now? As Ilja van Sprundel says, Xorg guys should steal that code! ;). Our Xorg binary is still setuid, but dropping the setuid bit isn't a problem in itself. What you care about is dropping as many access rights as possible, and being setuid you might actually be able to drop more of them. It also means you can open the traditional log files (either before you drop priviliges, or through the Xorg process that keeps the priviliges. But the major benefit from the privsep is that non-KMS setups still get (some of) the benefits. Not sure if Matthieu Herrb ever tried to merge the OpenBSD changes back into Xorg. If he did his attempts didn't raise much interest from you Linux people. But it seems that has changed ;). His changes can be found in the OpenBSD xenocara CVS repository. And if you ask him nicely, he can probably point you at a git repository that has the changes as well. Just grep for X_PRIVSEP! ___ 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
From: Hans de Goede hdego...@redhat.com Date: Wed, 5 Mar 2014 16:51:52 +0100 If you end up going with this wrapper approach anyway despite my previous message, here are some comments. Oh, and it's good that this is optional. 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) POSIX doesn't specify readlink(1), so it might not be available on all systems. Solaris doesn't seem to have it. Perhaps just have configure substitute the installation path here? +basedir=$(dirname $canonical) +if [ -x $basedir/Xorg.wrap ]; then + exec $basedir/Xorg.wrap $@ +else + exec $basedir/Xorg.bin $@ +fi diff --git a/hw/xfree86/xorg-wrapper.c b/hw/xfree86/xorg-wrapper.c new file mode 100644 index 000..93a5f15 --- /dev/null +++ b/hw/xfree86/xorg-wrapper.c @@ -0,0 +1,82 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: Hans de Goede hdego...@redhat.com + */ + +#include fcntl.h +#include limits.h +#include stdint.h +#include stdio.h +#include stdlib.h +#include string.h +#include sys/ioctl.h +#include sys/stat.h +#include sys/types.h +#include unistd.h +#include drm/drm.h + +#include dix-config.h /* For PROJECTROOT */ + +int main(int argc, char *argv[]) +{ +struct drm_mode_card_res res; +char buf[PATH_MAX]; +int i, r, fd; +int kms_cards = 0; +int total_cards = 0; + +for (i = 0; i 16; i++) { +snprintf(buf, PATH_MAX, /dev/dri/card%d, i); Hardcoding paths like this is a bad idea. We use /dev/drm%d on OpenBSD for example. Other systems might use different names yet again. Probably better to use libdrm. +fd = open(buf, O_RDWR); +if (fd == -1) +continue; + +total_cards++; + +memset(res, 0, sizeof(struct drm_mode_card_res)); +r = ioctl(fd, DRM_IOCTL_MODE_GETRESOURCES, res); +if (r == 0 res.count_connectors 0) +kms_cards++; + +close(fd); +} + +/* If we've found cards, and all cards support kms, drop root rights */ +if (total_cards kms_cards == total_cards) { I think total_cards only includes devices that have a kernel drm driver loaded. So what you're really checking here is how many of those provide KMS support. So you're missing any true legacy device. ___ 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 0/1] Xorg: Add a suid root wrapper
Hi Mark, On 03/06/2014 01:23 PM, Mark Kettenis wrote: snip Oh dear, the wrapper script is back! Before you go further down this road, may I point out the privilege seperation support that we've had in xenocara (Xorg for OpenBSD) for years now? As Ilja van Sprundel says, Xorg guys should steal that code! ;). Our Xorg binary is still setuid, but dropping the setuid bit isn't a problem in itself. Ideally it would not be suid at all, but agreed that that is not the biggest problem. What you care about is dropping as many access rights as possible, and being setuid you might actually be able to drop more of them. That sounds like nonsense to me, unless you're API's are broken somewhere you should be able drop capabilities / whatever just as well as regular user. Root should only ever be required to gain rights, never to drop them. It also means you can open the traditional log files (either before you drop priviliges, or through the Xorg process that keeps the priviliges. Agreed that this would allow us to open the log files, but as you indicate yourself, that is not a big deal. But the major benefit from the privsep is that non-KMS setups still get (some of) the benefits. Maybe, my main reason for needing a wrapper at all when running in no-root-rights normally needed mode are: 1) iopl / memmap rights needed by non kms drivers 2) support for binary only drivers Esp. in the second category we simply don't know what rights are needed an what rights can be dropped. From the Linux POV the wrapper is a workaround for these drivers, I hope that in the future we can stop shipping the wrapper by default. Not sure if Matthieu Herrb ever tried to merge the OpenBSD changes back into Xorg. If he did his attempts didn't raise much interest from you Linux people. But it seems that has changed ;). His changes can be found in the OpenBSD xenocara CVS repository. And if you ask him nicely, he can probably point you at a git repository that has the changes as well. Just grep for X_PRIVSEP! If there is interest in the OpenBSD community in getting this upstream the first step would be for someone interested in this to hack it up in a bunch of incremental patches (which may already be the case) and submitting those to the list for review. 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: [PATCH RFC] Xorg: Add a suid root wrapper
Hi, Thanks for the review! On 03/06/2014 01:46 PM, Mark Kettenis wrote: From: Hans de Goede hdego...@redhat.com Date: Wed, 5 Mar 2014 16:51:52 +0100 If you end up going with this wrapper approach anyway despite my previous message, here are some comments. Oh, and it's good that this is optional. 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) POSIX doesn't specify readlink(1), so it might not be available on all systems. Solaris doesn't seem to have it. Perhaps just have configure substitute the installation path here? Yeah that seems like a good idea. +basedir=$(dirname $canonical) +if [ -x $basedir/Xorg.wrap ]; then +exec $basedir/Xorg.wrap $@ +else +exec $basedir/Xorg.bin $@ +fi diff --git a/hw/xfree86/xorg-wrapper.c b/hw/xfree86/xorg-wrapper.c new file mode 100644 index 000..93a5f15 --- /dev/null +++ b/hw/xfree86/xorg-wrapper.c @@ -0,0 +1,82 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: Hans de Goede hdego...@redhat.com + */ + +#include fcntl.h +#include limits.h +#include stdint.h +#include stdio.h +#include stdlib.h +#include string.h +#include sys/ioctl.h +#include sys/stat.h +#include sys/types.h +#include unistd.h +#include drm/drm.h + +#include dix-config.h /* For PROJECTROOT */ + +int main(int argc, char *argv[]) +{ +struct drm_mode_card_res res; +char buf[PATH_MAX]; +int i, r, fd; +int kms_cards = 0; +int total_cards = 0; + +for (i = 0; i 16; i++) { +snprintf(buf, PATH_MAX, /dev/dri/card%d, i); Hardcoding paths like this is a bad idea. We use /dev/drm%d on OpenBSD for example. Other systems might use different names yet again. We may end up putting a define in some config.h for this, but ... Probably better to use libdrm. Ugh no, this is a suid-root wrapper, less code is more here. Yes I know libdrm is designed to be safe to run as root, but it is still better to not run it as root at all. +fd = open(buf, O_RDWR); +if (fd == -1) +continue; + +total_cards++; + +memset(res, 0, sizeof(struct drm_mode_card_res)); +r = ioctl(fd, DRM_IOCTL_MODE_GETRESOURCES, res); +if (r == 0 res.count_connectors 0) +kms_cards++; + +close(fd); +} + +/* If we've found cards, and all cards support kms, drop root rights */ +if (total_cards kms_cards == total_cards) { I think total_cards only includes devices that have a kernel drm driver loaded. So what you're really checking here is how many of those provide KMS support. So you're missing any true legacy device. That is why the check is there for if we've found any cards at all, so on a machine with only a legacy card the wrapper will keep the root rights. I admit that on a machine with 2 cards, one legacy one kms it may end up doing the wrong thing. That is why the non RFC version of this patch is going to have a /etc/X11/Xwrapper.config config file, so that people can simply put a needs_root_rights = 1 In there, given that this is rather a corner case which will likely require manual config anyways that seems like a better idea then complicating the wrapper a lot for this corner case. 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
[PATCH 3/6] glamor: Add a note about the state of GL_ARB_map_buffer_range.
GLES2 Xephyr is failing due to lack of glMapBuffer() with the read bits set, and I decided to see if we can just switch everything to glMapBufferRange(). I'm undecided, and it largely depends on whether we find people are interested in using glamor for the windows X server. Signed-off-by: Eric Anholt e...@anholt.net --- glamor/glamor.c | 13 + 1 file changed, 13 insertions(+) diff --git a/glamor/glamor.c b/glamor/glamor.c index fa753bb..fe9f761 100644 --- a/glamor/glamor.c +++ b/glamor/glamor.c @@ -319,6 +319,19 @@ glamor_init(ScreenPtr screen, unsigned int flags) gl_version = glamor_gl_get_version(); +/* We'd like to require GL_ARB_map_buffer_range or + * GL_OES_map_buffer_range, since it offers more information to + * the driver than plain old glMapBuffer() or glBufferSubData(). + * It's been supported on Mesa on the desktop since 2009 and on + * GLES2 since October 2012. It's supported on Apple's iOS + * drivers for SGX535 and A7, but apparently not on most Android + * devices (the OES extension spec wasn't released until June + * 2012). + * + * 82% of 0 A.D. players (desktop GL) submitting hardware reports + * have support for it, with most of the ones lacking it being on + * Windows with Intel 4-series (G45) graphics or older. + */ if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) { if (gl_version GLAMOR_GL_VERSION_ENCODE(1, 3)) { ErrorF(Require OpenGL version 1.3 or later.\n); -- 1.9.0 ___ 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
[PATCH 5/6] glamor: Fix a mismatched glamor_get/put_context().
We don't call GL in this function any more, so we can just drop the get. Signed-off-by: Eric Anholt e...@anholt.net --- glamor/glamor_gradient.c | 5 - 1 file changed, 5 deletions(-) diff --git a/glamor/glamor_gradient.c b/glamor/glamor_gradient.c index 9f6f1b1..6a7b528 100644 --- a/glamor/glamor_gradient.c +++ b/glamor/glamor_gradient.c @@ -46,8 +46,6 @@ static const char * _glamor_create_getcolor_fs_source(ScreenPtr screen, int stops_count, int use_array) { -glamor_screen_private *glamor_priv; - char *gradient_fs = NULL; #define gradient_fs_getcolor\ @@ -174,9 +172,6 @@ _glamor_create_getcolor_fs_source(ScreenPtr screen, int stops_count, return gradient_color;\n }\n; -glamor_priv = glamor_get_screen_private(screen); -glamor_get_context(glamor_priv); - if (use_array) { XNFasprintf(gradient_fs, gradient_fs_getcolor, stops_count, stops_count); -- 1.9.0 ___ 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
[PATCH 4/6] dix: Remove incorrect comment about privates.
PRIVATE_ALL was apparently dropped before this comment was added in commit 495fc3eb2d6c98bde82ae1278f89fcf131fd9bf8. Signed-off-by: Eric Anholt e...@anholt.net --- dix/privates.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dix/privates.c b/dix/privates.c index e03b225..dd26c6f 100644 --- a/dix/privates.c +++ b/dix/privates.c @@ -325,11 +325,10 @@ grow_screen_specific_set(DevPrivateType type, unsigned bytes) /* * Register a private key. This takes the type of object the key will - * be used with, which may be PRIVATE_ALL indicating that this key - * will be used with all of the private objects. If 'size' is - * non-zero, then the specified amount of space will be allocated in - * the private storage. Otherwise, space for a single pointer will - * be allocated which can be set with dixSetPrivate + * be used. If 'size' is non-zero, then the specified amount of space + * will be allocated in the private storage. Otherwise, space for a + * single pointer will be allocated which can be set with + * dixSetPrivate */ Bool dixRegisterPrivateKey(DevPrivateKey key, DevPrivateType type, unsigned size) -- 1.9.0 ___ 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
[PATCH 2/6] glamor: Fix a spelling mistake in GLAMOR_PIXMAP_FBO_NOT_EXACT_SIZE.
Signed-off-by: Eric Anholt e...@anholt.net --- glamor/glamor_pixmap.c | 2 +- glamor/glamor_render.c | 2 +- glamor/glamor_utils.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/glamor/glamor_pixmap.c b/glamor/glamor_pixmap.c index f1440f3..119e4d9 100644 --- a/glamor/glamor_pixmap.c +++ b/glamor/glamor_pixmap.c @@ -1528,7 +1528,7 @@ glamor_fixup_pixmap_priv(ScreenPtr screen, glamor_pixmap_private *pixmap_priv) drawable = pixmap_priv-base.pixmap-drawable; -if (!GLAMOR_PIXMAP_FBO_NOT_EAXCT_SIZE(pixmap_priv)) +if (!GLAMOR_PIXMAP_FBO_NOT_EXACT_SIZE(pixmap_priv)) return TRUE; old_fbo = pixmap_priv-base.fbo; diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 093a215..f8d103d 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -584,7 +584,7 @@ glamor_set_composite_texture(glamor_screen_private *glamor_priv, int unit, else if (glamor_priv-gl_flavor == GLAMOR_GL_ES2 || pixmap_priv-type == GLAMOR_TEXTURE_LARGE) { if (picture-transform -|| (GLAMOR_PIXMAP_FBO_NOT_EAXCT_SIZE(pixmap_priv))) +|| (GLAMOR_PIXMAP_FBO_NOT_EXACT_SIZE(pixmap_priv))) repeat_type += RepeatFix; } if (repeat_type = RepeatFix) { diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index 9374c9d..f9550b7 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -53,7 +53,7 @@ *(_pyscale_) = 1.0 / (_pixmap_priv_)-base.fbo-height; \ } while(0) -#define GLAMOR_PIXMAP_FBO_NOT_EAXCT_SIZE(priv) \ +#define GLAMOR_PIXMAP_FBO_NOT_EXACT_SIZE(priv) \ (priv-base.fbo-width != priv-base.pixmap-drawable.width \ || priv-base.fbo-height != priv-base.pixmap-drawable.height) \ -- 1.9.0 ___ 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
[PATCH 6/6] xephyr: Don't forget to glViewport() before drawing the screen.
Fixes misrendering with cairogears. I had noticed the failure while trying to figure out what was going on with traps. Cairogears was apparently putting its results on the screen through putimage, which is a texture upload, so the last GL drawing was done to the size of the cairogears window, not the size of the xephyr screen. --- hw/kdrive/ephyr/ephyr_glamor_glx.c | 15 +++ hw/kdrive/ephyr/ephyr_glamor_glx.h | 10 ++ hw/kdrive/ephyr/hostx.c| 4 3 files changed, 29 insertions(+) diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.c b/hw/kdrive/ephyr/ephyr_glamor_glx.c index d56907f..eaf5654 100644 --- a/hw/kdrive/ephyr/ephyr_glamor_glx.c +++ b/hw/kdrive/ephyr/ephyr_glamor_glx.c @@ -67,6 +67,9 @@ struct ephyr_glamor { GLuint texture_shader; GLuint texture_shader_position_loc; GLuint texture_shader_texcoord_loc; + +/* Size of the window that we're rendering to. */ +unsigned width, height; }; static GLint @@ -205,6 +208,7 @@ ephyr_glamor_damage_redisplay(struct ephyr_glamor *glamor, glBindFramebuffer(GL_FRAMEBUFFER, 0); glUseProgram(glamor-texture_shader); +glViewport(0, 0, glamor-width, glamor-height); glVertexAttribPointer(glamor-texture_shader_position_loc, 2, GL_FLOAT, FALSE, 0, position); @@ -329,3 +333,14 @@ ephyr_glamor_get_visual(void) return xcb_aux_find_visual_by_id(xscreen, visual_info-visualid); } + +void +ephyr_glamor_set_window_size(struct ephyr_glamor *glamor, + unsigned width, unsigned height) +{ +if (!glamor) +return; + +glamor-width = width; +glamor-height = height; +} diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.h b/hw/kdrive/ephyr/ephyr_glamor_glx.h index 8995e1e..0c238cf 100644 --- a/hw/kdrive/ephyr/ephyr_glamor_glx.h +++ b/hw/kdrive/ephyr/ephyr_glamor_glx.h @@ -51,6 +51,10 @@ ephyr_glamor_glx_screen_fini(struct ephyr_glamor *glamor); #ifdef GLAMOR void +ephyr_glamor_set_window_size(struct ephyr_glamor *glamor, + unsigned width, unsigned height); + +void ephyr_glamor_damage_redisplay(struct ephyr_glamor *glamor, struct pixman_region16 *damage); @@ -60,6 +64,12 @@ ephyr_glamor_process_event(xcb_generic_event_t *xev); #else /* !GLAMOR */ static inline void +ephyr_glamor_set_window_size(struct ephyr_glamor *glamor, + unsigned width, unsigned height) +{ +} + +static inline void ephyr_glamor_damage_redisplay(struct ephyr_glamor *glamor, struct pixman_region16 *damage) { diff --git a/hw/kdrive/ephyr/hostx.c b/hw/kdrive/ephyr/hostx.c index 19c2ed2..66019a3 100644 --- a/hw/kdrive/ephyr/hostx.c +++ b/hw/kdrive/ephyr/hostx.c @@ -728,6 +728,8 @@ hostx_screen_init(KdScreenInfo *screen, if (ephyr_glamor) { *bytes_per_line = 0; *bits_per_pixel = 0; +ephyr_glamor_set_window_size(scrpriv-glamor, + scrpriv-win_width, scrpriv-win_height); return NULL; } else if (host_depth_matches_server(scrpriv)) { *bytes_per_line = scrpriv-ximg-stride; @@ -1213,6 +1215,8 @@ ephyr_glamor_init(ScreenPtr screen) EphyrScrPriv *scrpriv = kd_screen-driver; scrpriv-glamor = ephyr_glamor_glx_screen_init(scrpriv-win); +ephyr_glamor_set_window_size(scrpriv-glamor, + scrpriv-win_width, scrpriv-win_height); glamor_init(screen, GLAMOR_USE_SCREEN | -- 1.9.0 ___ 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
little glamor cleanups (and a dix cleanup)
Here are some trivial patches from the glamor-server branch for review. I'm planning to send out the VBO changes next, which get us closer to making glamor not actually suck for performance. These are on the glamor-cleanups branch of my tree, branched from glamor-pull-request. ___ 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
[PATCH 1/6] glamor: remove dead global variable.
Signed-off-by: Eric Anholt e...@anholt.net --- glamor/glamor_pixmap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/glamor/glamor_pixmap.c b/glamor/glamor_pixmap.c index 77197b5..f1440f3 100644 --- a/glamor/glamor_pixmap.c +++ b/glamor/glamor_pixmap.c @@ -697,7 +697,6 @@ glamor_color_convert_to_bits(void *src_bits, void *dst_bits, int w, int h, * Upload pixmap to a specified texture. * This texture may not be the one attached to it. **/ -int in_restore = 0; static void __glamor_upload_pixmap_to_texture(PixmapPtr pixmap, unsigned int *tex, GLenum format, -- 1.9.0 ___ 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/ 6/14 04:46 AM, Mark Kettenis wrote: +canonical=$(readlink -f $0) POSIX doesn't specify readlink(1), so it might not be available on all systems. Solaris doesn't seem to have it. Perhaps just have configure substitute the installation path here? Solaris 11 and later have it, and I no longer spend effort on making new Xorg releases run on Solaris 10 and older. (As far as I know they still should mostly run on Solaris 8 and later, but I don't have the time nor motivation to ensure that, and have seen no one else submitting patches to do so.) That said, build time substitution is probably the better answer anyway. -- -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: [PULL] glamor xephyr and xorg changes
Eric Anholt e...@anholt.net writes: 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. Merged. b634e90..da08316 master - master -- keith.pack...@intel.com pgpVnsB4hsNMk.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
[PATCH xmodmap] Add support for hyphen as placeholder for preserving a current keysym
So far, a keycode or keysym command always overwrote the entire list of keysyms associated with a keycode. However, users often want to modify only some of the keysyms in the list, e.g. only the keysyms assigned when the Mode_switch key is pressed. With this patch, it is now possible to reassign some symkeys without affecting others on the same key, by listing a - where no change is requested. Example: Previously, if I wanted to have twosuperior (²) assigned to the key 2 when Mode_shift is pressed, I had to write xmodmap -e keysym 2 = 2 quotedbl twosuperior NoSymbol on a UK keyboard, but xmodmap -e keysym 2 = 2 at twosuperior NoSymbol on a US keyboard, which is tedious where multiple layouts are used. After this patch, the simpler command xmodmap -e keysym 2 = - - twosuperior NoSymbol will work on both UK and US keyboards alike. --- handle.c| 43 --- man/xmodmap.man |2 ++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/handle.c b/handle.c index 3f05a46..cc18c33 100644 --- a/handle.c +++ b/handle.c @@ -51,6 +51,8 @@ static XModifierKeymap *map = NULL; struct wq work_queue = {NULL, NULL}; +/* placeholder for a KeySym that should not be changed */ +#define KeepSymbol ((KeySym) -1) /* * common utility routines @@ -304,6 +306,10 @@ parse_keysym(const char *line, int n, char **name, KeySym *keysym) *keysym = NoSymbol; return (True); } +if (!strcmp(*name, -)) { + *keysym = KeepSymbol; + return (True); +} *keysym = XStringToKeysym (*name); if (*keysym == NoSymbol '0' = **name **name = '9') return parse_number(*name, keysym); @@ -1213,12 +1219,14 @@ execute_work_queue (void) static int exec_keycode(struct op_keycode *opk) { -if (!opk-target_keycode) { - int i, j; - KeyCode free; +KeyCode keycode = opk-target_keycode; +int i, old_count = 0; +KeySym *old_keysyms = NULL; + +if (!keycode) { + int j; if (!opk-count) return (0); - free = 0; for (i = min_keycode; i = max_keycode; i++) { for (j = 0; j opk-count; j++) { if (XKeycodeToKeysym(dpy, (KeyCode) i, j) != opk-keysyms[j]) @@ -1226,29 +1234,42 @@ exec_keycode(struct op_keycode *opk) } if (j = opk-count) return (0); - if (free) + if (keycode) continue; for (j = 0; j 8; j++) { if (XKeycodeToKeysym(dpy, (KeyCode) i, j) != None) break; } if (j = 8) - free = i; + keycode = i; } - if (!free) { + if (!keycode) { fprintf(stderr, %s: no available keycode for assignment\n, ProgramName); return (-1); } - XChangeKeyboardMapping (dpy, free, opk-count, opk-keysyms, 1); } else if (opk-count == 0) { KeySym dummy = NoSymbol; XChangeKeyboardMapping (dpy, opk-target_keycode, 1, dummy, 1); -} else { - XChangeKeyboardMapping (dpy, opk-target_keycode, opk-count, - opk-keysyms, 1); + return (0); } + +/* replace any KeepSymbol keysyms with fetched previous value */ +for (i = 0; i opk-count; i++) { + if (opk-keysyms[i] == KeepSymbol) { + if (!old_keysyms) + old_keysyms = XGetKeyboardMapping (dpy, keycode, 1, old_count); + if (old_keysyms i old_count) + opk-keysyms[i] = old_keysyms[i]; + else + opk-keysyms[i] = NoSymbol; + } +} +if (old_keysyms) XFree(old_keysyms); + +XChangeKeyboardMapping (dpy, keycode, opk-count, opk-keysyms, 1); + return (0); } diff --git a/man/xmodmap.man b/man/xmodmap.man index 1ed1ca1..fcb7773 100644 --- a/man/xmodmap.man +++ b/man/xmodmap.man @@ -153,6 +153,8 @@ are not used in any major X server implementation. The first keysym is used when no modifier key is pressed in conjunction with this key, the second with Shift, the third when the Mode_switch key is used with this key and the fourth when both the Mode_switch and Shift keys are used. +A hyphen instead of a keysym name keeps the keysym currently assigned +at that position. .TP 8 .B keycode any = \fIKEYSYMNAME ...\fP If no existing key has the specified list of keysyms assigned to it, -- 1.7.9.5 ___ 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/6] dix: Remove incorrect comment about privates.
Am 2014-03-06 18:00, schrieb Eric Anholt: PRIVATE_ALL was apparently dropped before There wasn't any PRIVATE_ALL key neither in xserver nor in glamor. I guess it's just a wrong reference to privates.h, so it's the same as the enum DevPrivateKey itself. ___ 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 3/6] glamor: Add a note about the state of GL_ARB_map_buffer_range.
Am 2014-03-06 18:00, schrieb Eric Anholt: +/* We'd like to require GL_ARB_map_buffer_range or OpenGL ES 3 or + * GL_OES_map_buffer_range, since it offers more information to + * the driver than plain old glMapBuffer() or glBufferSubData(). + * It's been supported on Mesa on the desktop since 2009 and on + * GLES2 since October 2012. It's supported on Apple's iOS + * drivers for SGX535 and A7, but apparently not on most Android + * devices (the OES extension spec wasn't released until June + * 2012). + * + * 82% of 0 A.D. players (desktop GL) submitting hardware reports + * have support for it, with most of the ones lacking it being on + * Windows with Intel 4-series (G45) graphics or older. + */ if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) { if (gl_version GLAMOR_GL_VERSION_ENCODE(1, 3)) { ErrorF(Require OpenGL version 1.3 or later.\n); glMapBuffer was dropped in OpenGL ES3, so we also have to check for this version to enable glMapBufferRange. But keep in mind that most current mobile drivers doesn't support unsync mapping. ___ 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: little glamor cleanups (and a dix cleanup)
Patches 1-5 are: Reviewed-by: Markus Wick markus at selfnet.de Am 2014-03-06 18:00, schrieb Eric Anholt: Here are some trivial patches from the glamor-server branch for review. I'm planning to send out the VBO changes next, which get us closer to making glamor not actually suck for performance. These are on the glamor-cleanups branch of my tree, branched from glamor-pull-request. ___ 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] dix: Clear any existing selections before initializing privates
Andrew Eikum aei...@codeweavers.com writes: Hi, we're less than a month out from the end of the 1.16 merge window[1]. Thanks for the reminder, and sorry for not getting it merged earlier! da08316..78e508c master - master -- keith.pack...@intel.com pgpiR6_F7UBnb.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
[PATCH] glx: Clear new FBConfig attributes to 0 by default.
The visualSelectGroup wasn't getting set (since our DRI drivers don't use it), and and since it's the top priority in the sort order, you got random sorting of your visuals unless malloc really returned you new memory. This manifested as Xephyr -glamor rendering to a multisampled window on my system, which as you might guess was slightly lower performance than expected. Signed-off-by: Eric Anholt e...@anholt.net --- glx/glxdricommon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c index 69d4b29..62cce13 100644 --- a/glx/glxdricommon.c +++ b/glx/glxdricommon.c @@ -132,7 +132,7 @@ createModeFromConfig(const __DRIcoreExtension * core, unsigned int attrib, value; int i; -config = malloc(sizeof *config); +config = calloc(1, sizeof *config); config-driConfig = driConfig; -- 1.9.0 ___ 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] glx: Clear new FBConfig attributes to 0 by default.
On Don, 2014-03-06 at 19:07 -0800, Eric Anholt wrote: The visualSelectGroup wasn't getting set (since our DRI drivers don't use it), and and since it's the top priority in the sort order, you got random sorting of your visuals unless malloc really returned you new memory. This manifested as Xephyr -glamor rendering to a multisampled window on my system, which as you might guess was slightly lower performance than expected. Signed-off-by: Eric Anholt e...@anholt.net Reviewed-by: Michel Dänzer michel.daen...@amd.com -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ 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: [PULL 1.15] First set of 1.15 fixes
Merged. Updating 2ea973e..f41ab8c On 02/18/2014 06:37 PM, Peter Hutterer wrote: Not sure if you still want to be the stable maintainer, but if so: here's your first set of cherry-picks. Mostly general cleanups that I found ont he branch, Carlos' patch fixes a crash with some tablet devices under GNOME, and the button state check is a mild security bug. The following changes since commit 2ea973e12f5d954211e1d10085a4c74581b43aca: Bump version to 1.15.0 (2013-12-27 09:50:55 -0800) are available in the git repository at: git://people.freedesktop.org/~whot/xserver server-1.15-branch for you to fetch changes up to f41ab8c60780ea8f87354e536e5b73cb23878eb7: dix: prevent a driver from initializing or submitting buttons MAX_BUTTONS (2014-02-19 10:25:00 +1000) Alan Coopersmith (3): Check for calloc() failure in add_master() On realloc failure, free font_path_string instead of leaking it xf86DeleteScreen: move check for NULL pScrn before first dereference Carlos Garnacho (1): Xi: Ensure DeviceChanged is emitted after grabs are deactivated Peter Hutterer (4): dix: fix button state check before changing a button mapping os: restrict display names to digits Xi: fix modifier offset in XIPassiveGrab swapping function dix: prevent a driver from initializing or submitting buttons MAX_BUTTONS Xi/exevents.c | 19 ++- Xi/xichangehierarchy.c | 4 Xi/xipassivegrab.c | 2 +- dix/devices.c | 1 + dix/dixfonts.c | 9 ++--- dix/getevents.c | 2 ++ dix/inpututils.c| 3 ++- hw/xfree86/common/xf86Helper.c | 7 --- os/utils.c | 27 +++ test/xi2/protocol-xipassivegrabdevice.c | 9 - 10 files changed, 73 insertions(+), 10 deletions(-) -- Matt Dew mar...@osource.org Key signature: 0xF7C3 BEC3 Fingerprint: FDB1 9D94 C573 DC29 BCCB 2F9F A6BF 3771 F7C3 BEC3 signature.asc Description: OpenPGP digital 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: [PULL 1.14 and 1.15] XQuartz Hardening
1.14 merged. 200f988..4ab897c server-1.14-branch - server-1.14-branch 1.15 merged. f41ab8c..b332cd2 server-1.15-branch - server-1.15-branch On 01/22/2014 04:22 PM, Jeremy Huddleston Sequoia wrote: The 1.14 pull request is below. Please also pull changes for 1.15 when that branch opens up, at git://people.freedesktop.org/~jeremyhu/xserver server-1.15-branch --- The following changes since commit 200f98894a43114586eb2d4405e766e8a4a59221: bump version from 1.14.4.901 to 1.14.5 (2013-12-12 20:51:27 -0700) are available in the git repository at: git://people.freedesktop.org/~jeremyhu/xserver server-1.14-branch for you to fetch changes up to 4ab897c03389d5e6e23a4ef7f3f6a4fda1837a53: XQuartz: Avoid passing uninitialized pointers to X11ApplicationSetWindowMenu in AppleWMSetWindowMenu (2014-01-22 15:17:49 -0800) Jeremy Huddleston Sequoia (12): darwin: Don't leave stdin/stdout closed XQuartz: Use asl_log_descriptor to log stdout/stderr of child processes XQuartz: Silence some static analyzer warnings by annotating referencing counts XQuartz: Fix darwinfb.h header guard XQuartz: Mark applicationWillTerminate: noreturn XQuartz: Simplify hook_run to quiet static analyzer XQuartz: Validate screen in AppleDRIQueryDirectRenderingCapable requests XQuartz: Validate length in appledri before swapping XQuartz: Silence a clang static analysis warning about a possible memory leak on exit XQuartz: Silence a clang static analysis warning about a memory leak XQuartz: Check for allocated memory before using it in AppleWMSetWindowMenu XQuartz: Avoid passing uninitialized pointers to X11ApplicationSetWindowMenu in AppleWMSetWindowMenu hw/xquartz/X11Application.m| 13 + hw/xquartz/X11Controller.m | 10 +- hw/xquartz/applewm.c | 16 hw/xquartz/darwinfb.h | 2 +- hw/xquartz/mach-startup/stub.c | 4 hw/xquartz/quartz.c| 3 +++ hw/xquartz/xpr/appledri.c | 10 ++ hw/xquartz/xpr/x-hook.c| 27 ++- os/osinit.c| 13 +++-- 9 files changed, 69 insertions(+), 29 deletions(-) -- Matt Dew mar...@osource.org Key signature: 0xF7C3 BEC3 Fingerprint: FDB1 9D94 C573 DC29 BCCB 2F9F A6BF 3771 F7C3 BEC3 signature.asc Description: OpenPGP digital 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