Re: ioctl cleanups: move SG_IO translation where it belongs

From: Andi Kleen (ak@suse.de)
Date: Tue May 06 2003 - 15:21:11 EST


On Tue, 2003-05-06 at 22:03, Pavel Machek wrote:
> Hi!
>
> This enables sharing of 200 lines of SG_IO support by all 64-bit
> architectures. If it looks okay, more such patches will follow.

I currently have some patches for this function pending. When an
unchanged data buffer is passed it is ok to just verify_area it, no need
to kmalloc and copy. This simplifies this handler vastly.

Here is the part from the 2.4 patch; haven't tried it with 2.5 yet,
but should apply there too.

Also adds some boundary checking.

-Andi

Index: linux-work/arch/x86_64/ia32/ia32_ioctl.c
===================================================================
RCS file: /home/cvs/Repository/linux/arch/x86_64/ia32/ia32_ioctl.c,v
retrieving revision 1.31
diff -u -u -r1.31 ia32_ioctl.c
--- linux-work/arch/x86_64/ia32/ia32_ioctl.c 2003/03/21 07:50:07 1.31
+++ linux-work/arch/x86_64/ia32/ia32_ioctl.c 2003/04/26 16:38:39
@@ -1373,12 +1381,16 @@
         u32 iov_len;
 } sg_iovec32_t;
 
+#define EMU_SG_MAX 128
+
 static int alloc_sg_iovec(sg_io_hdr_t *sgp, u32 uptr32)
 {
         sg_iovec32_t *uiov = (sg_iovec32_t *) A(uptr32);
         sg_iovec_t *kiov;
         int i;
 
+ if (sgp->iovec_count > EMU_SG_MAX)
+ return -EINVAL;
         sgp->dxferp = kmalloc(sgp->iovec_count *
                               sizeof(sg_iovec_t), GFP_KERNEL);
         if (!sgp->dxferp)
@@ -1391,40 +1403,10 @@
                 u32 iov_base32;
                 if (__get_user(iov_base32, &uiov->iov_base) ||
                     __get_user(kiov->iov_len, &uiov->iov_len))
- return -EFAULT;
-
- kiov->iov_base = kmalloc(kiov->iov_len, GFP_KERNEL);
- if (!kiov->iov_base)
- return -ENOMEM;
- if (copy_from_user(kiov->iov_base,
- (void *) A(iov_base32),
- kiov->iov_len))
- return -EFAULT;
-
- uiov++;
- kiov++;
- }
-
- return 0;
-}
-
-static int copy_back_sg_iovec(sg_io_hdr_t *sgp, u32 uptr32)
-{
- sg_iovec32_t *uiov = (sg_iovec32_t *) A(uptr32);
- sg_iovec_t *kiov = (sg_iovec_t *) sgp->dxferp;
- int i;
-
- for (i = 0; i < sgp->iovec_count; i++) {
- u32 iov_base32;
-
- if (__get_user(iov_base32, &uiov->iov_base))
                         return -EFAULT;
-
- if (copy_to_user((void *) A(iov_base32),
- kiov->iov_base,
- kiov->iov_len))
+ if (verify_area(VERIFY_WRITE, (void *)A(iov_base32), kiov->iov_len))
                         return -EFAULT;
-
+ kiov->iov_base = (void *)A(iov_base32);
                 uiov++;
                 kiov++;
         }
@@ -1434,16 +1416,6 @@
 
 static void free_sg_iovec(sg_io_hdr_t *sgp)
 {
- sg_iovec_t *kiov = (sg_iovec_t *) sgp->dxferp;
- int i;
-
- for (i = 0; i < sgp->iovec_count; i++) {
- if (kiov->iov_base) {
- kfree(kiov->iov_base);
- kiov->iov_base = NULL;
- }
- kiov++;
- }
         kfree(sgp->dxferp);
         sgp->dxferp = NULL;
 }
@@ -1506,6 +1483,11 @@
                         goto out;
                 }
         } else {
+ if (sg_io64.dxfer_len > 4*PAGE_SIZE) {
+ err = -EINVAL;
+ goto out;
+ }
+
                 sg_io64.dxferp = kmalloc(sg_io64.dxfer_len, GFP_KERNEL);
                 if (!sg_io64.dxferp) {
                         err = -ENOMEM;
@@ -1546,7 +1528,7 @@
         err |= copy_to_user((void *)A(sbp32), sg_io64.sbp, sg_io64.mx_sb_len);
         if (sg_io64.dxferp) {
                 if (sg_io64.iovec_count)
- err |= copy_back_sg_iovec(&sg_io64, dxferp32);
+ ;
                 else
                         err |= copy_to_user((void *)A(dxferp32),
                                             sg_io64.dxferp,

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



This archive was generated by hypermail 2b29 : Wed May 07 2003 - 22:00:28 EST