Attachment 132308 Details for Bug 101595 – Fix to prevent keymap trashing

[patch] Fix to prevent keymap trashing

0002-input-Send-unique-keymap-file-descriptors-to-clients.patch (text/plain), 4.58 KB, created by Derek Foreman on 2017-06-28 18:26:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Derek Foreman

Created: 2017-06-28 18:26:21 UTC

Size: 4.58 KB

patch

obsolete

>From dfda6107f4693f09cb28602ee68c40ff5cadcb29 Mon Sep 17 00:00:00 2001
>From: Derek Foreman <derekf@osg.samsung.com>
>Date: Wed, 28 Jun 2017 12:17:46 -0500
>Subject: [PATCH weston 2/2] input: Send unique keymap file descriptors to
> clients
>
>Client may map any file descriptor opened for writing with PROT_WRITE
>themselves.  On linux, even a read-only file descriptor to an unlinked
>file can be re-opened with write permission through /proc/self/fd.
>
>The only way to prevent this is to create a memfd which
>is subsequently write-sealed.  Unfortunately this prevents clients
>from mapping with MAP_SHARED, which is already in widespread usage.
>
>To isolate and protect the keymap, whilst allowing MAP_SHARED clients
>to continue to work, use a unique file descriptor for each
>wl_keyboard resource.
>
>Reviewed-by: Daniel Stone <daniels@collabora.com>
>Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
>---
> libweston/compositor.h |  3 +--
> libweston/input.c      | 60 +++++++++++++++++++++++---------------------------
> 2 files changed, 28 insertions(+), 35 deletions(-)
>
>diff --git a/libweston/compositor.h b/libweston/compositor.h
>index 2c3358c5..ecfd030c 100644
>--- a/libweston/compositor.h
>+++ b/libweston/compositor.h
>@@ -547,9 +547,8 @@ weston_touch_start_drag(struct weston_touch *touch,
> 
> struct weston_xkb_info {
> 	struct xkb_keymap *keymap;
>-	int keymap_fd;
> 	size_t keymap_size;
>-	char *keymap_area;
>+	char *keymap_string;
> 	int32_t ref_count;
> 	xkb_mod_index_t shift_mod;
> 	xkb_mod_index_t caps_mod;
>diff --git a/libweston/input.c b/libweston/input.c
>index 001d7b19..4d562afc 100644
>--- a/libweston/input.c
>+++ b/libweston/input.c
>@@ -1856,11 +1856,31 @@ WL_EXPORT void
> weston_keyboard_send_keymap(struct weston_keyboard *kbd, struct wl_resource *resource)
> {
> 	struct weston_xkb_info *xkb_info = kbd->xkb_info;
>+	void *area;
>+	int fd;
> 
>+	fd = os_create_anonymous_file(xkb_info->keymap_size);
>+	if (fd < 0) {
>+		weston_log("creating a keymap file for %lu bytes failed: %m\n",
>+			   (unsigned long) xkb_info->keymap_size);
>+		return;
>+	}
>+
>+	area = mmap(NULL, xkb_info->keymap_size, PROT_READ | PROT_WRITE,
>+		    MAP_SHARED, fd, 0);
>+	if (area == MAP_FAILED) {
>+		weston_log("failed to mmap() %lu bytes\n",
>+			   (unsigned long) xkb_info->keymap_size);
>+		goto err_mmap;
>+	}
>+	strcpy(area, xkb_info->keymap_string);
>+	munmap(area, xkb_info->keymap_size);
> 	wl_keyboard_send_keymap(resource,
> 				WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
>-				xkb_info->keymap_fd,
>+				fd,
> 				xkb_info->keymap_size);
>+err_mmap:
>+	close(fd);
> }
> 
> static void
>@@ -2679,10 +2699,8 @@ weston_xkb_info_destroy(struct weston_xkb_info *xkb_info)
> 
> 	xkb_keymap_unref(xkb_info->keymap);
> 
>-	if (xkb_info->keymap_area)
>-		munmap(xkb_info->keymap_area, xkb_info->keymap_size);
>-	if (xkb_info->keymap_fd >= 0)
>-		close(xkb_info->keymap_fd);
>+	if (xkb_info->keymap_string)
>+		free(xkb_info->keymap_string);
> 	free(xkb_info);
> }
> 
>@@ -2710,8 +2728,6 @@ weston_xkb_info_create(struct xkb_keymap *keymap)
> 	xkb_info->keymap = xkb_keymap_ref(keymap);
> 	xkb_info->ref_count = 1;
> 
>-	char *keymap_str;
>-
> 	xkb_info->shift_mod = xkb_keymap_mod_get_index(xkb_info->keymap,
> 						       XKB_MOD_NAME_SHIFT);
> 	xkb_info->caps_mod = xkb_keymap_mod_get_index(xkb_info->keymap,
>@@ -2736,38 +2752,16 @@ weston_xkb_info_create(struct xkb_keymap *keymap)
> 	xkb_info->scroll_led = xkb_keymap_led_get_index(xkb_info->keymap,
> 							XKB_LED_NAME_SCROLL);
> 
>-	keymap_str = xkb_keymap_get_as_string(xkb_info->keymap,
>-					      XKB_KEYMAP_FORMAT_TEXT_V1);
>-	if (keymap_str == NULL) {
>+	xkb_info->keymap_string = xkb_keymap_get_as_string(xkb_info->keymap,
>+							   XKB_KEYMAP_FORMAT_TEXT_V1);
>+	if (xkb_info->keymap_string == NULL) {
> 		weston_log("failed to get string version of keymap\n");
> 		goto err_keymap;
> 	}
>-	xkb_info->keymap_size = strlen(keymap_str) + 1;
>-
>-	xkb_info->keymap_fd = os_create_anonymous_file(xkb_info->keymap_size);
>-	if (xkb_info->keymap_fd < 0) {
>-		weston_log("creating a keymap file for %lu bytes failed: %m\n",
>-			(unsigned long) xkb_info->keymap_size);
>-		goto err_keymap_str;
>-	}
>-
>-	xkb_info->keymap_area = mmap(NULL, xkb_info->keymap_size,
>-				     PROT_READ | PROT_WRITE,
>-				     MAP_SHARED, xkb_info->keymap_fd, 0);
>-	if (xkb_info->keymap_area == MAP_FAILED) {
>-		weston_log("failed to mmap() %lu bytes\n",
>-			(unsigned long) xkb_info->keymap_size);
>-		goto err_dev_zero;
>-	}
>-	strcpy(xkb_info->keymap_area, keymap_str);
>-	free(keymap_str);
>+	xkb_info->keymap_size = strlen(xkb_info->keymap_string) + 1;
> 
> 	return xkb_info;
> 
>-err_dev_zero:
>-	close(xkb_info->keymap_fd);
>-err_keymap_str:
>-	free(keymap_str);
> err_keymap:
> 	xkb_keymap_unref(xkb_info->keymap);
> 	free(xkb_info);
>-- 
>2.13.1
>

Actions: View | Splinter Review

Attachments on bug 101595: 132248 | 132254 | 132291 | 132307 | 132308 | 132327 | 132375

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.