lkml.org 
[lkml]   [2002]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Subject2.4.18 bugs
Date

2.4.18

below are a few bugs leading to reading kernel memory using some of the usb
drivers.

--
Silvio

--

drivers/usb/se401.h

struct usb_se401 {

[ skip ]

int *width;
int *height;
int cwidth; /* current width */
int cheight; /* current height */

every integer in this structure (and .h) is signed, irrespective of its
usage. the char pointers are unsigned in places though.

:(

./drivers/usb/se401.c

static int se401_set_size(struct usb_se401 *se401, int width, int height)
{
int wasstreaming=se401->streaming;
/* Check to see if we need to change */
if (se401->cwidth==width && se401->cheight==height)
return 0;

/* Check for a valid mode */
if (!width || !height)
return 1;
if ((width & 1) || (height & 1))
return 1;
if (width>se401->width[se401->sizes-1])
return 1;
if (height>se401->height[se401->sizes-1])
return 1;

/* Stop a current stream and start it again at the new size */
if (wasstreaming)
se401_stop_stream(se401);
se401->cwidth=width;
se401->cheight=height;

width / height can be modified (to a negative for instance) - something
might break though with this though --> (will check more later).

static long se401_read(struct video_device *dev, char *buf, unsigned long count, int noblock)
{
int realcount=count, ret=0;
struct usb_se401 *se401 = (struct usb_se401 *)dev;

if (se401->dev == NULL)
return -EIO;
if (realcount > se401->cwidth*se401->cheight*3)
realcount=se401->cwidth*se401->cheight*3;

[ skip ]

if (copy_to_user(buf, se401->frame[0].data, realcount))
return -EFAULT;

sign and overflow problem, leading to unbounded copy_to_user.

--

./drivers/usb/usbvideo.c

long usbvideo_v4l_read(struct video_device *dev, char *buf, unsigned long count, int noblock)
{

[ skip ]

/*
* Copy bytes to user space. We allow for partial reads, which
* means that the user application can request read less than
* the full frame size. It is up to the application to issue
* subsequent calls until entire frame is read.
*
* First things first, make sure we don't copy more than we
* have - even if the application wants more. That would be
* a big security embarassment!
*/
if ((count + frame->seqRead_Index) > frame->seqRead_Length)
count = frame->seqRead_Length - frame->seqRead_Index;

/*
* Copy requested amount of data to user space. We start
* copying from the position where we last left it, which
* will be zero for a new frame (not read before).
*/
if (copy_to_user(buf, frame->data + frame->seqRead_Index, count)) {
count = -EFAULT;
goto read_done;
}

count + frame->seqRead_Index can integer overflow and then buffer overflow
in copy_to_user.

--

./drivers/usb/vicam.c


static int vicam_init(struct usb_vicam *vicam)
{
int width[] = {128, 256, 512};
int height[] = {122, 242, 242};

dbg("vicam_init");
buf = kmalloc(0x1e480, GFP_KERNEL);
buf2 = kmalloc(0x1e480, GFP_KERNEL);

static long vicam_v4l_read(struct video_device *vdev, char *user_buf, unsigned long buflen, int noblock)
{
//struct usb_vicam *vicam = (struct usb_vicam *)vdev;

dbg("vicam_v4l_read(%ld)", buflen);

if (!vdev || !buf)
return -EFAULT;

if (copy_to_user(user_buf, buf2, buflen))
return -EFAULT;
return buflen;
}

is this crazy? i was thinking this was impossible (ie, upper layer checking
for it), but other drivers have explicit checks..

am i crazy here? (i still think i am).

* First things first, make sure we don't copy more than we
* have - even if the application wants more. That would be
* a big security embarassment!
*/

from the other sources above ;-) (which has an integer and sign overflows)

--

./net/x25/af_x25.c


len = min_t(unsigned int, len, sizeof(int));

if (len < 0)
return -EINVAL;

the len < 0 check is always false.

^^ silly pedant that i am.

--
Silvio

Communicate in total privacy.
Get your free encrypted email at https://www.hushmail.com/?l=2

Looking for a good deal on a domain name? http://www.hush.com/partners/offers.cgi?id=domainpeople

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:27    [W:0.053 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site