2004-10-10 00:04:39

by Jesper Juhl

Subject: [PATCH] check copy_to_user return value in raw1394

Here's a proposed patch to make sure we check the return value of
copy_to_user in raw1394.c::raw1394_read
I've changed __copy_to_user into copy_to_user since I don't see where we
would otherwhise be doing the access_ok checking...
Please review this patch before applying.

Signed-off-by: Jesper Juhl <[email protected]>

diff -up linux-2.6.9-rc3-bk9-orig/drivers/ieee1394/raw1394.c linux-2.6.9-rc3-bk9/drivers/ieee1394/raw1394.c
--- linux-2.6.9-rc3-bk9-orig/drivers/ieee1394/raw1394.c 2004-09-30 05:03:45.000000000 +0200
+++ linux-2.6.9-rc3-bk9/drivers/ieee1394/raw1394.c 2004-10-10 02:05:54.000000000 +0200
@@ -411,6 +411,7 @@ static ssize_t raw1394_read(struct file
struct file_info *fi = (struct file_info *)file->private_data;
struct list_head *lh;
struct pending_request *req;
+ ssize_t ret;

if (count != sizeof(struct raw1394_request)) {
return -EINVAL;
@@ -443,10 +444,15 @@ static ssize_t raw1394_read(struct file
req->req.error = RAW1394_ERROR_MEMFAULT;
}
}
- __copy_to_user(buffer, &req->req, sizeof(req->req));
+ if(copy_to_user(buffer, &req->req, sizeof(req->req))) {
+ ret = -EFAULT;
+ goto out;
+ }

+ ret = (ssize_t)sizeof(struct raw1394_request);
+out:
free_pending_request(req);
- return sizeof(struct raw1394_request);
+ return ret;
}

2004-10-10 00:15:11

by Jesper Juhl

Subject: Re: [PATCH] check copy_to_user return value in raw1394

On Sun, 10 Oct 2004, Jesper Juhl wrote:

>
> Here's a proposed patch to make sure we check the return value of
> copy_to_user in raw1394.c::raw1394_read


Whoops, I made an error when I set the From: address on this mail. If you
reply to this then please use juhl-lkml as the address if you want me to
see your reply.

--
Jesper Juhl <[email protected]>

2004-10-10 00:21:28

by Randy.Dunlap

Subject: Re: [PATCH] check copy_to_user return value in raw1394

Jesper Juhl wrote:
> On Sun, 10 Oct 2004, Jesper Juhl wrote:
>
>
>>Here's a proposed patch to make sure we check the return value of
>>copy_to_user in raw1394.c::raw1394_read
>
>
>
> Whoops, I made an error when I set the From: address on this mail. If you
> reply to this then please use juhl-lkml as the address if you want me to
> see your reply.

How about sending it to:

IEEE 1394 SUBSYSTEM
P: Ben Collins
M: [email protected]
L: [email protected]
W: http://www.linux1394.org/
S: Maintained

and change "if(" to "if (" ...

--
~Randy

2004-10-10 02:22:33

by Jesper Juhl

Subject: Re: [PATCH] check copy_to_user return value in raw1394

On Sat, 9 Oct 2004, Randy.Dunlap wrote:

> Jesper Juhl wrote:
> > On Sun, 10 Oct 2004, Jesper Juhl wrote:
> >
> >
> > > Here's a proposed patch to make sure we check the return value of
> > > copy_to_user in raw1394.c::raw1394_read
> >
>
> How about sending it to:
>
> IEEE 1394 SUBSYSTEM
> P: Ben Collins
> M: [email protected]
> L: [email protected]
> W: http://www.linux1394.org/
> S: Maintained
>

Right, I should probably do that... Added as a recipient on this mail...

> and change "if(" to "if (" ...
>
Done.


Here's a revised patch :

Jesper Juhl <[email protected]>

diff -up linux-2.6.9-rc3-bk9-orig/drivers/ieee1394/raw1394.c linux-2.6.9-rc3-bk9/drivers/ieee1394/raw1394.c
--- linux-2.6.9-rc3-bk9-orig/drivers/ieee1394/raw1394.c 2004-09-30 05:03:45.000000000 +0200
+++ linux-2.6.9-rc3-bk9/drivers/ieee1394/raw1394.c 2004-10-10 04:24:57.000000000 +0200
@@ -411,6 +411,7 @@ static ssize_t raw1394_read(struct file
struct file_info *fi = (struct file_info *)file->private_data;
struct list_head *lh;
struct pending_request *req;
+ ssize_t ret;

if (count != sizeof(struct raw1394_request)) {
return -EINVAL;
@@ -443,10 +444,15 @@ static ssize_t raw1394_read(struct file
req->req.error = RAW1394_ERROR_MEMFAULT;
}
}
- __copy_to_user(buffer, &req->req, sizeof(req->req));
+ if (copy_to_user(buffer, &req->req, sizeof(req->req))) {
+ ret = -EFAULT;
+ goto out;
+ }

+ ret = (ssize_t)sizeof(struct raw1394_request);
+out:
free_pending_request(req);
- return sizeof(struct raw1394_request);
+ return ret;
}