mbox

Karmic SRU: thinkpad-acpi: lock down video output state access, CVE-2010-3448

Message ID 20110124194538.D73C8F89F8@sepang.rtg.net
State Accepted
Headers show

Pull-request

git://kernel.ubuntu.com/rtg/ubuntu-karmic.git CVE-2010-3448

Message

Tim Gardner Jan. 24, 2011, 7:45 p.m. UTC 
The following changes since commit 09f654208690b182c9c4691aa47e7b7c87325969:
  Henrique de Moraes Holschuh (1):
        thinkpad-acpi: lock down video output state access, CVE-2010-3448

are available in the git repository at:

  git://kernel.ubuntu.com/rtg/ubuntu-karmic.git CVE-2010-3448


From 09f654208690b182c9c4691aa47e7b7c87325969 Mon Sep 17 00:00:00 2001
From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date: Mon, 24 Jan 2011 08:11:01 -0700
Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448

BugLink: http://bugs.launchpad.net/bugs/706999

Back ported from commit b525c06cdbd8a3963f0173ccd23f9147d4c384b5 upstream
by Tim Gardner <tim.gardner@canonical.com>. Resolves CVE-2010-3448

Given the right combination of ThinkPad and X.org, just reading the
video output control state is enough to hard-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.

This bug could be considered a local DoS I suppose, as it allows any
non-privledged local user to cause some versions of X.org to
hard-crash some ThinkPads.

Reported-by: Jidanni <jidanni@jidanni.org>
Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 Documentation/laptops/thinkpad-acpi.txt |    4 ++++
 drivers/platform/x86/Kconfig            |   10 ++++++++--
 drivers/platform/x86/thinkpad_acpi.c    |   18 +++++++++++++++---
 3 files changed, 27 insertions(+), 5 deletions(-)

Comments

Stefan Bader Jan. 25, 2011, 10:37 a.m. UTC | #1 
On 01/24/2011 08:45 PM, Tim Gardner wrote:
> The following changes since commit 09f654208690b182c9c4691aa47e7b7c87325969:
>   Henrique de Moraes Holschuh (1):
>         thinkpad-acpi: lock down video output state access, CVE-2010-3448
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-karmic.git CVE-2010-3448
> 
> 
> From 09f654208690b182c9c4691aa47e7b7c87325969 Mon Sep 17 00:00:00 2001
> From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Date: Mon, 24 Jan 2011 08:11:01 -0700
> Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448
> 
CVE-2010-3448
> BugLink: http://bugs.launchpad.net/bugs/706999
> 
> Back ported from commit b525c06cdbd8a3963f0173ccd23f9147d4c384b5 upstream
> by Tim Gardner <tim.gardner@canonical.com>. Resolves CVE-2010-3448

I probably would put the backported statement just above your sob in the sob
area and the resolves normally is handled by just putting the cve number at
column 1 (like added above).

> 
> Given the right combination of ThinkPad and X.org, just reading the
> video output control state is enough to hard-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.
> 
> This bug could be considered a local DoS I suppose, as it allows any
> non-privledged local user to cause some versions of X.org to
> hard-crash some ThinkPads.
> 
> Reported-by: Jidanni <jidanni@jidanni.org>
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
[maybe?](cherry-picker|backported from commit <sha1> upstream)
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  Documentation/laptops/thinkpad-acpi.txt |    4 ++++
>  drivers/platform/x86/Kconfig            |   10 ++++++++--
>  drivers/platform/x86/thinkpad_acpi.c    |   18 +++++++++++++++---
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
> index e2ddcde..fa12ef8 100644
> --- a/Documentation/laptops/thinkpad-acpi.txt
> +++ b/Documentation/laptops/thinkpad-acpi.txt
> @@ -674,6 +674,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 77c6097..bbe9036 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -290,9 +290,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 05e5d56..a83803f 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -235,6 +235,7 @@ struct ibm_init_struct {
>  	char param[32];
>  
>  	int (*init) (struct ibm_init_struct *);
> +	mode_t base_procfs_mode;
>  	struct ibm_struct *data;
>  };
>  
> @@ -4142,6 +4143,10 @@ static int video_read(char *p)
>  		return len;
>  	}
>  
> +	/* Even reads can crash X.org, so... */
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
>  	status = video_outputsw_get();
>  	if (status < 0)
>  		return status;
> @@ -4175,6 +4180,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;
>  
> @@ -7357,9 +7366,11 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
>  		"%s installed\n", ibm->name);
>  
>  	if (ibm->read) {
> -		entry = create_proc_entry(ibm->name,
> -					  S_IFREG | S_IRUGO | S_IWUSR,
> -					  proc_dir);
> +		mode_t mode = iibm->base_procfs_mode;
> +
> +		if (!mode)
> +			mode = S_IRUGO;
> +		entry = create_proc_entry(ibm->name, mode, proc_dir);
>  		if (!entry) {
>  			printk(TPACPI_ERR "unable to create proc entry %s\n",
>  			       ibm->name);
> @@ -7555,6 +7566,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
Brad Figg Jan. 27, 2011, 5:08 p.m. UTC | #2 
On 01/25/2011 02:37 AM, Stefan Bader wrote:
> On 01/24/2011 08:45 PM, Tim Gardner wrote:
>> The following changes since commit 09f654208690b182c9c4691aa47e7b7c87325969:
>>    Henrique de Moraes Holschuh (1):
>>          thinkpad-acpi: lock down video output state access, CVE-2010-3448
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-karmic.git CVE-2010-3448
>>
>>
>>  From 09f654208690b182c9c4691aa47e7b7c87325969 Mon Sep 17 00:00:00 2001
>> From: Henrique de Moraes Holschuh<hmh@hmh.eng.br>
>> Date: Mon, 24 Jan 2011 08:11:01 -0700
>> Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448
>>
> CVE-2010-3448
>> BugLink: http://bugs.launchpad.net/bugs/706999
>>
>> Back ported from commit b525c06cdbd8a3963f0173ccd23f9147d4c384b5 upstream
>> by Tim Gardner<tim.gardner@canonical.com>. Resolves CVE-2010-3448
>
> I probably would put the backported statement just above your sob in the sob
> area and the resolves normally is handled by just putting the cve number at
> column 1 (like added above).
>
>>
>> Given the right combination of ThinkPad and X.org, just reading the
>> video output control state is enough to hard-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.
>>
>> This bug could be considered a local DoS I suppose, as it allows any
>> non-privledged local user to cause some versions of X.org to
>> hard-crash some ThinkPads.
>>
>> Reported-by: Jidanni<jidanni@jidanni.org>
>> Signed-off-by: Henrique de Moraes Holschuh<hmh@hmh.eng.br>
>> Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de>
> [maybe?](cherry-picker|backported from commit<sha1>  upstream)
>> Signed-off-by: Tim Gardner<tim.gardner@canonical.com>
> Acked-by: Stefan Bader<stefan.bader@canonical.com>
>> ---
>>   Documentation/laptops/thinkpad-acpi.txt |    4 ++++
>>   drivers/platform/x86/Kconfig            |   10 ++++++++--
>>   drivers/platform/x86/thinkpad_acpi.c    |   18 +++++++++++++++---
>>   3 files changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
>> index e2ddcde..fa12ef8 100644
>> --- a/Documentation/laptops/thinkpad-acpi.txt
>> +++ b/Documentation/laptops/thinkpad-acpi.txt
>> @@ -674,6 +674,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 77c6097..bbe9036 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -290,9 +290,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 05e5d56..a83803f 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -235,6 +235,7 @@ struct ibm_init_struct {
>>   	char param[32];
>>
>>   	int (*init) (struct ibm_init_struct *);
>> +	mode_t base_procfs_mode;
>>   	struct ibm_struct *data;
>>   };
>>
>> @@ -4142,6 +4143,10 @@ static int video_read(char *p)
>>   		return len;
>>   	}
>>
>> +	/* Even reads can crash X.org, so... */
>> +	if (!capable(CAP_SYS_ADMIN))
>> +		return -EPERM;
>> +
>>   	status = video_outputsw_get();
>>   	if (status<  0)
>>   		return status;
>> @@ -4175,6 +4180,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;
>>
>> @@ -7357,9 +7366,11 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
>>   		"%s installed\n", ibm->name);
>>
>>   	if (ibm->read) {
>> -		entry = create_proc_entry(ibm->name,
>> -					  S_IFREG | S_IRUGO | S_IWUSR,
>> -					  proc_dir);
>> +		mode_t mode = iibm->base_procfs_mode;
>> +
>> +		if (!mode)
>> +			mode = S_IRUGO;
>> +		entry = create_proc_entry(ibm->name, mode, proc_dir);
>>   		if (!entry) {
>>   			printk(TPACPI_ERR "unable to create proc entry %s\n",
>>   			       ibm->name);
>> @@ -7555,6 +7566,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
>
>

Acked-by: Brad Figg <brad.figg@canonical.com>
Tim Gardner Jan. 27, 2011, 5:29 p.m. UTC | #3 
applied and pushed