xf86-video-intel: 5 commits - src/backlight.c tools/backlight_helper.c

[Chris Wilson]

 src/backlight.c          |   18 +++++++++++++-----
 tools/backlight_helper.c |   29 +++++++----------------------
 2 files changed, 20 insertions(+), 27 deletions(-)

New commits:
commit e860b3eaedcb5d56745d5bcc9ce720e9a0e9c293
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Mon Feb 17 13:16:54 2014 +0100

    backlight-helper: Simplify reading the level from stdin
    
    Since the helper is a standalone app, the usual xserver rules of not using
    stdio because of signal handling don't apply.
    
    And since the helper does run with elevated rights, it is important to keep
    the code KISS so that it can be audited easily.
    
    This commit replaces the hard to read "raw" read loop with a much simpler
    loop using fgets, improving readability of the code.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/tools/backlight_helper.c b/tools/backlight_helper.c
index fc16fce..11abebc 100644
--- a/tools/backlight_helper.c
+++ b/tools/backlight_helper.c
@@ -9,7 +9,7 @@
 int main(int argc, char *argv[])
 {
 	struct stat st;
-	char buf[1024], *b = buf;
+	char buf[1024];
 	int len, fd;
 
 	if (argc != 2) {
@@ -24,27 +24,12 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	while ((len = read(0, b, sizeof(buf) - (b - buf) - 1)) > 0) {
-		len += b - buf;
-		buf[len] = '\0';
-
-		b = buf;
-		do {
-			char *end = strchr(b, '\n');
-			if (end == NULL)
-				break;
-
-			++end;
-			if (write(fd, b, end - b) != end - b) {
-				fprintf(stderr, "Failed to update backlight interface '%s'\n", argv[1]);
-				return 2;
-			}
-
-			b = end;
-		} while (1);
-
-		memmove(buf, b, len = buf + len - b);
-		b = buf + len;
+	while (fgets(buf, sizeof(buf), stdin)) {
+		len = strlen(buf);
+		if (write(fd, buf, len) != len) {
+			fprintf(stderr, "Failed to update backlight interface '%s'\n", argv[1]);
+			return 2;
+		}
 	}
 
 	return 0;
commit b5229c6e15964e723a7bb99d3c2c20a2fa168e3b
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Mon Feb 17 13:16:53 2014 +0100

    backlight: Drop rights before executing pkexec
    
    Event though we've failed to open the backlight normally, we may still be
    running under a suid-root xserver, so drop any elevated rights before
    executing what we hope will be pkxec.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/src/backlight.c b/src/backlight.c
index 0501070..688819d 100644
--- a/src/backlight.c
+++ b/src/backlight.c
@@ -295,9 +295,14 @@ static int __backlight_helper_init(struct backlight *b, char *iface)
 
 	switch ((b->pid = fork())) {
 	case 0:
+		if (setgid(getgid()) || setuid(getuid()))
+			_exit(127);
+
 		close(fds[1]);
-		dup2(fds[0], 0);
+		if (dup2(fds[0], 0))
+			_exit(127);
 		close(fds[0]);
+
 		if (use_pkexec) {
 			execlp("pkexec", "pkexec",
 			       LIBEXEC_PATH "/xf86-video-intel-backlight-helper",
commit 27a9dc4ce8fa3ba56f68e04d11a6272348772039
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Mon Feb 17 13:16:52 2014 +0100

    backlight: Use System instead of system when checking for pkexec
    
    Even though we've failed to open the backlight normally, we may still be
    running under a suid-root xserver, so use the servers build in System instead
    of system so as to properly drop root rights.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/src/backlight.c b/src/backlight.c
index 728cdda..0501070 100644
--- a/src/backlight.c
+++ b/src/backlight.c
@@ -41,6 +41,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <dirent.h>
+#include <xf86.h>
 
 #include "backlight.h"
 #include "fd.h"
@@ -283,7 +284,7 @@ static int __backlight_helper_init(struct backlight *b, char *iface)
 		return 0;
 
 	if ((st.st_mode & (S_IFREG | S_ISUID | S_IXUSR)) != (S_IFREG | S_ISUID | S_IXUSR)) {
-		if (system("pkexec --version"))
+		if (System("pkexec --version"))
 			return 0;
 
 		use_pkexec = 1;
commit 06066b7269b5ccf502c27466ad0434adfd9e3866
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Mon Feb 17 13:16:51 2014 +0100

    backlight: Explain better why we support both pkexec and suid root for the helper
    
    Update the comment about trying suid-root first with some explanations of
    why pkexec may be preferable in some cases.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/src/backlight.c b/src/backlight.c
index b756929..728cdda 100644
--- a/src/backlight.c
+++ b/src/backlight.c
@@ -270,8 +270,10 @@ static int __backlight_helper_init(struct backlight *b, char *iface)
 	int use_pkexec = 0;
 	int fds[2];
 
-	/* If system policy is to disallow setuid helpers,
-	 * we fallback to invoking PolicyKit. However, as pkexec
+	/*
+	 * Some systems may prefer using PolicyKit's pkexec over
+	 * making the helper suid root, since the suid option will allow
+	 * anyone to control the backlight.  However, as pkexec
 	 * is quite troublesome and not universally available, we
 	 * still try the old fashioned and simple method first.
 	 * Either way, we have to trust that it is our backlight-helper
commit 6b5c2b4515b646db6be0f9307f53651620516521
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Sat Feb 15 21:03:49 2014 +0000

    backlight: tidy use of BACKLIGHT_CLASS
    
    Use string concantenation to simply the sprintf slightly.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/backlight.c b/src/backlight.c
index 70c6559..b756929 100644
--- a/src/backlight.c
+++ b/src/backlight.c
@@ -143,7 +143,7 @@ __backlight_open(const char *iface, const char *file, int mode)
 	char buf[1024];
 	int fd;
 
-	snprintf(buf, sizeof(buf), "%s/%s/%s", BACKLIGHT_CLASS, iface, file);
+	snprintf(buf, sizeof(buf), BACKLIGHT_CLASS "/%s/%s", iface, file);
 	fd = open(buf, mode);
 	if (fd == -1)
 		return -1;