Patchwork [Karmic,CVE-2010-4074,1/1] USB: serial/mos*: prevent reading uninitialized stack memory

login
register
mail settings
Submitter Brad Figg
Date Jan. 24, 2011, 6:57 p.m.
Message ID <1295895438-1507-2-git-send-email-brad.figg@canonical.com>
Download mbox | patch
Permalink /patch/80232/
State Accepted
Delegated to: Stefan Bader
Headers show

Comments

Brad Figg - Jan. 24, 2011, 6:57 p.m.
From: Dan Rosenberg <drosenberg@vsecurity.com>

CVE-2010-4074

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

The TIOCGICOUNT device ioctl in both mos7720.c and mos7840.c allows
unprivileged users to read uninitialized stack memory, because the
"reserved" member of the serial_icounter_struct struct declared on the
stack is not altered or zeroed before being copied back to the user.
This patch takes care of it.

Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Brad Figg <brad.figg@canonical.com>
---
 drivers/usb/serial/mos7720.c |    3 +++
 drivers/usb/serial/mos7840.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)
Tim Gardner - Jan. 24, 2011, 8 p.m.
On 01/24/2011 11:57 AM, Brad Figg wrote:
> From: Dan Rosenberg<drosenberg@vsecurity.com>
>
> CVE-2010-4074
>
> BugLink: http://bugs.launchpad.net/bugs/706149
>
> The TIOCGICOUNT device ioctl in both mos7720.c and mos7840.c allows
> unprivileged users to read uninitialized stack memory, because the
> "reserved" member of the serial_icounter_struct struct declared on the
> stack is not altered or zeroed before being copied back to the user.
> This patch takes care of it.
>
> Signed-off-by: Dan Rosenberg<dan.j.rosenberg@gmail.com>
> Cc: stable<stable@kernel.org>
> Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de>
> Signed-off-by: Brad Figg<brad.figg@canonical.com>
> ---
>   drivers/usb/serial/mos7720.c |    3 +++
>   drivers/usb/serial/mos7840.c |    3 +++
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
> index ccd4dd3..6571077 100644
> --- a/drivers/usb/serial/mos7720.c
> +++ b/drivers/usb/serial/mos7720.c
> @@ -1431,6 +1431,9 @@ static int mos7720_ioctl(struct tty_struct *tty, struct file *file,
>
>   	case TIOCGICOUNT:
>   		cnow = mos7720_port->icount;
> +
> +		memset(&icount, 0, sizeof(struct serial_icounter_struct));
> +
>   		icount.cts = cnow.cts;
>   		icount.dsr = cnow.dsr;
>   		icount.rng = cnow.rng;
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 270009a..879bacb 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -2357,6 +2357,9 @@ static int mos7840_ioctl(struct tty_struct *tty, struct file *file,
>   	case TIOCGICOUNT:
>   		cnow = mos7840_port->icount;
>   		smp_rmb();
> +
> +		memset(&icount, 0, sizeof(struct serial_icounter_struct));
> +
>   		icount.cts = cnow.cts;
>   		icount.dsr = cnow.dsr;
>   		icount.rng = cnow.rng;

I'd like to see the upstream commit from which this patch was either 
cherry-picked or backported in the commit log. Its also of interest to 
know if it was actually accepted into one or more stable kernels. That 
provides some assurance that the patch has passed by some maintainer 
eyeballs. Otherwise,

Acked-by: Tim Gardner tim.gardner@canonical.com>
Stefan Bader - Jan. 25, 2011, 10:48 a.m.
On 01/24/2011 07:57 PM, Brad Figg wrote:
> From: Dan Rosenberg <drosenberg@vsecurity.com>
> 
> CVE-2010-4074
> 
> BugLink: http://bugs.launchpad.net/bugs/706149
> 
> The TIOCGICOUNT device ioctl in both mos7720.c and mos7840.c allows
> unprivileged users to read uninitialized stack memory, because the
> "reserved" member of the serial_icounter_struct struct declared on the
> stack is not altered or zeroed before being copied back to the user.
> This patch takes care of it.
> 
> Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Brad Figg <brad.figg@canonical.com>
Comments like Hardy version
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/usb/serial/mos7720.c |    3 +++
>  drivers/usb/serial/mos7840.c |    3 +++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
> index ccd4dd3..6571077 100644
> --- a/drivers/usb/serial/mos7720.c
> +++ b/drivers/usb/serial/mos7720.c
> @@ -1431,6 +1431,9 @@ static int mos7720_ioctl(struct tty_struct *tty, struct file *file,
>  
>  	case TIOCGICOUNT:
>  		cnow = mos7720_port->icount;
> +
> +		memset(&icount, 0, sizeof(struct serial_icounter_struct));
> +
>  		icount.cts = cnow.cts;
>  		icount.dsr = cnow.dsr;
>  		icount.rng = cnow.rng;
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 270009a..879bacb 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -2357,6 +2357,9 @@ static int mos7840_ioctl(struct tty_struct *tty, struct file *file,
>  	case TIOCGICOUNT:
>  		cnow = mos7840_port->icount;
>  		smp_rmb();
> +
> +		memset(&icount, 0, sizeof(struct serial_icounter_struct));
> +
>  		icount.cts = cnow.cts;
>  		icount.dsr = cnow.dsr;
>  		icount.rng = cnow.rng;
Tim Gardner - Jan. 25, 2011, 3:17 p.m.
On 01/24/2011 01:00 PM, Tim Gardner wrote:
> On 01/24/2011 11:57 AM, Brad Figg wrote:
>> From: Dan Rosenberg<drosenberg@vsecurity.com>
>>
>> CVE-2010-4074
>>
>> BugLink: http://bugs.launchpad.net/bugs/706149
>>
>> The TIOCGICOUNT device ioctl in both mos7720.c and mos7840.c allows
>> unprivileged users to read uninitialized stack memory, because the
>> "reserved" member of the serial_icounter_struct struct declared on the
>> stack is not altered or zeroed before being copied back to the user.
>> This patch takes care of it.
>>
>> Signed-off-by: Dan Rosenberg<dan.j.rosenberg@gmail.com>
>> Cc: stable<stable@kernel.org>
>> Signed-off-by: Greg Kroah-Hartman<gregkh@suse.de>
>> Signed-off-by: Brad Figg<brad.figg@canonical.com>
>> ---
>> drivers/usb/serial/mos7720.c | 3 +++
>> drivers/usb/serial/mos7840.c | 3 +++
>> 2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
>> index ccd4dd3..6571077 100644
>> --- a/drivers/usb/serial/mos7720.c
>> +++ b/drivers/usb/serial/mos7720.c
>> @@ -1431,6 +1431,9 @@ static int mos7720_ioctl(struct tty_struct *tty,
>> struct file *file,
>>
>> case TIOCGICOUNT:
>> cnow = mos7720_port->icount;
>> +
>> + memset(&icount, 0, sizeof(struct serial_icounter_struct));
>> +
>> icount.cts = cnow.cts;
>> icount.dsr = cnow.dsr;
>> icount.rng = cnow.rng;
>> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
>> index 270009a..879bacb 100644
>> --- a/drivers/usb/serial/mos7840.c
>> +++ b/drivers/usb/serial/mos7840.c
>> @@ -2357,6 +2357,9 @@ static int mos7840_ioctl(struct tty_struct *tty,
>> struct file *file,
>> case TIOCGICOUNT:
>> cnow = mos7840_port->icount;
>> smp_rmb();
>> +
>> + memset(&icount, 0, sizeof(struct serial_icounter_struct));
>> +
>> icount.cts = cnow.cts;
>> icount.dsr = cnow.dsr;
>> icount.rng = cnow.rng;
>
> I'd like to see the upstream commit from which this patch was either
> cherry-picked or backported in the commit log. Its also of interest to
> know if it was actually accepted into one or more stable kernels. That
> provides some assurance that the patch has passed by some maintainer
> eyeballs. Otherwise,
>
> Acked-by: Tim Gardner tim.gardner@canonical.com>
>

applied and pushed

Patch

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index ccd4dd3..6571077 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1431,6 +1431,9 @@  static int mos7720_ioctl(struct tty_struct *tty, struct file *file,
 
 	case TIOCGICOUNT:
 		cnow = mos7720_port->icount;
+
+		memset(&icount, 0, sizeof(struct serial_icounter_struct));
+
 		icount.cts = cnow.cts;
 		icount.dsr = cnow.dsr;
 		icount.rng = cnow.rng;
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 270009a..879bacb 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -2357,6 +2357,9 @@  static int mos7840_ioctl(struct tty_struct *tty, struct file *file,
 	case TIOCGICOUNT:
 		cnow = mos7840_port->icount;
 		smp_rmb();
+
+		memset(&icount, 0, sizeof(struct serial_icounter_struct));
+
 		icount.cts = cnow.cts;
 		icount.dsr = cnow.dsr;
 		icount.rng = cnow.rng;