mbox

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

Message ID 20110124194732.2C542F89F8@sepang.rtg.net
State Accepted
Headers show

Pull-request

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

Message

Tim Gardner Jan. 24, 2011, 7:47 p.m. UTC
The following changes since commit e98c9ee35cead70474b238604a6e9edaf91f7270:
  Jason Gaston (1):
        ata_piix: IDE mode SATA patch for Intel ICH10 DeviceID's

are available in the git repository at:

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

Tim Gardner (1):
      thinkpad-acpi: lock down video output state access, CVE-2010-3448

 Documentation/thinkpad-acpi.txt |    4 ++++
 drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
 drivers/misc/thinkpad_acpi.h    |    1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

From 66921b3a1779614924ab89c98f473523dfbe18fa Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Mon, 24 Jan 2011 10:43:57 -0700
Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448

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

Backported from 2.6.32.y 1b0d63f15fb79d0cb840f8b701f343548b5640e8.
I dropped part of the patch that applied to drivers/platform/x86/Kconfig
since its not relevant for 2.6.24.

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/thinkpad-acpi.txt |    4 ++++
 drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
 drivers/misc/thinkpad_acpi.h    |    1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

Comments

Stefan Bader Jan. 25, 2011, 10:43 a.m. UTC | #1
On 01/24/2011 08:47 PM, Tim Gardner wrote:
> The following changes since commit e98c9ee35cead70474b238604a6e9edaf91f7270:
>   Jason Gaston (1):
>         ata_piix: IDE mode SATA patch for Intel ICH10 DeviceID's
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/rtg/ubuntu-hardy.git CVE-2010-3448
> 
> Tim Gardner (1):
>       thinkpad-acpi: lock down video output state access, CVE-2010-3448
> 
>  Documentation/thinkpad-acpi.txt |    4 ++++
>  drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
>  drivers/misc/thinkpad_acpi.h    |    1 +
>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> From 66921b3a1779614924ab89c98f473523dfbe18fa Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Mon, 24 Jan 2011 10:43:57 -0700
> Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448
> 
CVE-2010-3448
> BugLink: http://bugs.launchpad.net/bugs/706999
> 
> Backported from 2.6.32.y 1b0d63f15fb79d0cb840f8b701f343548b5640e8.
> I dropped part of the patch that applied to drivers/platform/x86/Kconfig
> since its not relevant for 2.6.24.
> 
> 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>
Similar thoughts like for Karmic version. I often see upstream doing things like

(backported from commit 1b0d63f15fb79d0cb840f8b701f343548b5640e8 2.6.32.y)
[dropped part applying to drivers/platform/x86/Kconfig as non-relevant]
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  Documentation/thinkpad-acpi.txt |    4 ++++
>  drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
>  drivers/misc/thinkpad_acpi.h    |    1 +
>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
> index 10c041c..46ae816 100644
> --- a/Documentation/thinkpad-acpi.txt
> +++ b/Documentation/thinkpad-acpi.txt
> @@ -518,6 +518,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/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
> index cf56647..ac866b9 100644
> --- a/drivers/misc/thinkpad_acpi.c
> +++ b/drivers/misc/thinkpad_acpi.c
> @@ -1986,6 +1986,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;
> @@ -2019,6 +2023,10 @@ static int video_write(char *buf)
>  	if (video_supported == TPACPI_VIDEO_NONE)
>  		return -ENODEV;
>  
> +	/* Even reads can crash X.org, so... */
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
>  	enable = 0;
>  	disable = 0;
>  
> @@ -4574,9 +4582,12 @@ 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(IBM_ERR "unable to create proc entry %s\n",
>  			       ibm->name);
> @@ -4758,6 +4769,7 @@ static struct ibm_init_struct ibms_init[] __initdata = {
>  	},
>  	{
>  		.init = video_init,
> +		.base_procfs_mode = S_IRUSR,
>  		.data = &video_driver_data,
>  	},
>  	{
> diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
> index 8fba2bb..9649256 100644
> --- a/drivers/misc/thinkpad_acpi.h
> +++ b/drivers/misc/thinkpad_acpi.h
> @@ -230,6 +230,7 @@ struct ibm_init_struct {
>  	char param[32];
>  
>  	int (*init) (struct ibm_init_struct *);
> +	mode_t base_procfs_mode;
>  	struct ibm_struct *data;
>  };
>
Brad Figg Jan. 27, 2011, 5:09 p.m. UTC | #2
On 01/24/2011 11:47 AM, Tim Gardner wrote:
> The following changes since commit e98c9ee35cead70474b238604a6e9edaf91f7270:
>    Jason Gaston (1):
>          ata_piix: IDE mode SATA patch for Intel ICH10 DeviceID's
>
> are available in the git repository at:
>
>    git://kernel.ubuntu.com/rtg/ubuntu-hardy.git CVE-2010-3448
>
> Tim Gardner (1):
>        thinkpad-acpi: lock down video output state access, CVE-2010-3448
>
>   Documentation/thinkpad-acpi.txt |    4 ++++
>   drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
>   drivers/misc/thinkpad_acpi.h    |    1 +
>   3 files changed, 20 insertions(+), 3 deletions(-)
>
>  From 66921b3a1779614924ab89c98f473523dfbe18fa Mon Sep 17 00:00:00 2001
> From: Tim Gardner<tim.gardner@canonical.com>
> Date: Mon, 24 Jan 2011 10:43:57 -0700
> Subject: [PATCH] thinkpad-acpi: lock down video output state access, CVE-2010-3448
>
> BugLink: http://bugs.launchpad.net/bugs/706999
>
> Backported from 2.6.32.y 1b0d63f15fb79d0cb840f8b701f343548b5640e8.
> I dropped part of the patch that applied to drivers/platform/x86/Kconfig
> since its not relevant for 2.6.24.
>
> 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/thinkpad-acpi.txt |    4 ++++
>   drivers/misc/thinkpad_acpi.c    |   18 +++++++++++++++---
>   drivers/misc/thinkpad_acpi.h    |    1 +
>   3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
> index 10c041c..46ae816 100644
> --- a/Documentation/thinkpad-acpi.txt
> +++ b/Documentation/thinkpad-acpi.txt
> @@ -518,6 +518,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/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
> index cf56647..ac866b9 100644
> --- a/drivers/misc/thinkpad_acpi.c
> +++ b/drivers/misc/thinkpad_acpi.c
> @@ -1986,6 +1986,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;
> @@ -2019,6 +2023,10 @@ static int video_write(char *buf)
>   	if (video_supported == TPACPI_VIDEO_NONE)
>   		return -ENODEV;
>
> +	/* Even reads can crash X.org, so... */
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
>   	enable = 0;
>   	disable = 0;
>
> @@ -4574,9 +4582,12 @@ 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(IBM_ERR "unable to create proc entry %s\n",
>   			       ibm->name);
> @@ -4758,6 +4769,7 @@ static struct ibm_init_struct ibms_init[] __initdata = {
>   	},
>   	{
>   		.init = video_init,
> +		.base_procfs_mode = S_IRUSR,
>   		.data =&video_driver_data,
>   	},
>   	{
> diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
> index 8fba2bb..9649256 100644
> --- a/drivers/misc/thinkpad_acpi.h
> +++ b/drivers/misc/thinkpad_acpi.h
> @@ -230,6 +230,7 @@ struct ibm_init_struct {
>   	char param[32];
>
>   	int (*init) (struct ibm_init_struct *);
> +	mode_t base_procfs_mode;
>   	struct ibm_struct *data;
>   };
>

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