Re: Question on verify_area() and friends wrt

dan carpenter (d_carpenter@sbcglobal.net)
Sun, 25 May 2003 00:46:31 +0200


On Sunday 25 May 2003 03:53 pm, Paulo Andre' wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> > verify_area only does some checks so you need to check the return
> > value from copy_to_user. You could switch to __copy_to_user, though.
>
> Why would __copy_to_user be a good choice? AFAIK, __copy_to_user does no
> validy checks (as opposed to copy_to_user which does access_ok()) so,
> considering verify_area() does only some checks, one could argue that
> there's even less checking done if using __copy_to_user. Where am I
> interpreting this wrong (as I certainly am) ?
>

copy_to_user() does the equivelent of a verify_area(). __copy_to_user()
doesn't
make the verify_area() check.If a function is going to be making a lot of
copies to
the same area, it makes sense to just do one verify_area() and use
__copy_to_user().
Both copy_to_user() and __copy_to_user() can fail even though the
verify_area()
checks pass.

In this case there is only one copy to each area so it doesn't really make
sense to use __copy_to_user().

My patch would look like this:

--- net/bluetooth/hci_core.c.orig 2003-05-25 00:25:16.000000000 +0200
+++ net/bluetooth/hci_core.c 2003-05-25 00:25:34.000000000 +0200
@@ -431,14 +431,14 @@

BT_DBG("num_rsp %d", ir.num_rsp);

- if (!verify_area(VERIFY_WRITE, ptr, sizeof(ir) +
- (sizeof(struct inquiry_info) * ir.num_rsp))) {
- copy_to_user(ptr, &ir, sizeof(ir));
- ptr += sizeof(ir);
- copy_to_user(ptr, buf, sizeof(struct inquiry_info) *
ir.num_rsp);
- } else
+ if (copy_to_user(ptr, &ir, sizeof(ir))) {
err = -EFAULT;
-
+ goto free:
+ }
+ ptr += sizeof(ir);
+ if (copy_to_user(ptr, buf, sizeof(struct inquiry_info) * ir.num_rsp))
+ err = -EFAULT;
+free:
kfree(buf);

done:

-
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/