From 62ff92211d319820c88f3f49dcb93a7548a5f395 Mon Sep 17 00:00:00 2001 From: Henrique de Moraes Holschuh Date: Sun, 7 Feb 2010 21:52:58 -0200 Subject: [PATCH] thinkpad-acpi: lock down video output control to root only Jidani reports that, given the right combination of ThinkPad and X.org, just reading the video output control state can crash X.org. Until the day I somehow find out a model or BIOS cut date to not provide this feature to ThinkPads that can do video switching through X RandR, change permissions so that only processes with CAP_SYS_ADMIN can access any sort of video output control state. Reported-by: Jidani Signed-off-by: Henrique de Moraes Holschuh --- Documentation/laptops/thinkpad-acpi.txt | 4 ++++ drivers/platform/x86/Kconfig | 10 ++++++++-- drivers/platform/x86/thinkpad_acpi.c | 15 +++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt index 0d9629c3498..6a33beb40b2 100644 --- a/Documentation/laptops/thinkpad-acpi.txt +++ b/Documentation/laptops/thinkpad-acpi.txt @@ -650,6 +650,10 @@ LCD, CRT or DVI (if available). The following commands are available: echo expand_toggle > /proc/acpi/ibm/video echo video_switch > /proc/acpi/ibm/video +NOTE: Access to this feature is restricted to processes owning the +CAP_SYS_ADMIN capability for safety reasons, as it can interact badly +enough with some versions of X.org to crash it. + Each video output device can be enabled or disabled individually. Reading /proc/acpi/ibm/video shows the status of each device. diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index e2117d57398..68edd92ec55 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -291,9 +291,15 @@ config THINKPAD_ACPI_VIDEO server running, phase of the moon, and the current mood of Schroedinger's cat. If you can use X.org's RandR to control your ThinkPad's video output ports instead of this feature, - don't think twice: do it and say N here to save some memory. + don't think twice: do it and say N here to save memory and avoid + bad interactions with X.org. - If you are not sure, say Y here. + NOTE: access to this feature is limited to processes with the + CAP_SYS_ADMIN capability, to avoid local DoS issues in platforms + where it interacts badly with X.org. + + If you are not sure, say Y here but do try to check if you could + be using X.org RandR instead. config THINKPAD_ACPI_HOTKEY_POLL bool "Support NVRAM polling for hot keys" diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 2a3b13755ee..9302a067291 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -293,6 +293,7 @@ struct ibm_init_struct { char param[32]; int (*init) (struct ibm_init_struct *); + mode_t base_procfs_mode; struct ibm_struct *data; }; @@ -4617,6 +4618,10 @@ static int video_read(struct seq_file *m) return 0; } + /* Even reads can crash X.org, so... */ + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + status = video_outputsw_get(); if (status < 0) return status; @@ -4650,6 +4655,10 @@ static int video_write(char *buf) if (video_supported == TPACPI_VIDEO_NONE) return -ENODEV; + /* Even reads can crash X.org, let alone writes... */ + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + enable = 0; disable = 0; @@ -8475,9 +8484,10 @@ static int __init ibm_init(struct ibm_init_struct *iibm) "%s installed\n", ibm->name); if (ibm->read) { - mode_t mode; + mode_t mode = iibm->base_procfs_mode; - mode = S_IRUGO; + if (!mode) + mode = S_IRUGO; if (ibm->write) mode |= S_IWUSR; entry = proc_create_data(ibm->name, mode, proc_dir, @@ -8668,6 +8678,7 @@ static struct ibm_init_struct ibms_init[] __initdata = { #ifdef CONFIG_THINKPAD_ACPI_VIDEO { .init = video_init, + .base_procfs_mode = S_IRUSR, .data = &video_driver_data, }, #endif -- 2.11.4.GIT