Re: ioctl cleanups: move SG_IO translation where it belongs

Andi Kleen (ak@suse.de)
06 May 2003 22:21:11 +0200


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/