Message ID | 20110124194732.2C542F89F8@sepang.rtg.net |
---|---|
State | Accepted |
Headers | show |
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; > }; >
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>
applied and pushed