From 7a80bfcd1f4bac61d586d3551f74215ff02e9cba Mon Sep 17 00:00:00 2001 From: vibi sreenivasan Date: Thu, 4 Jun 2009 20:59:17 +0530 Subject: [PATCH] Staging: rspiusb: copy_to/from_user related fixes The patch does copy_to/from_user related fixes *) __copy_from/to_user is enough for user space data buffer checked by access_ok. *) return -EFAULT if __copy_from/to_user fails. *) Do not use memcpy to copy from user space. Signed-off-by: Vibi Sreenivasan Signed-off-by: Greg Kroah-Hartman --- drivers/staging/rspiusb/rspiusb.c | 44 +++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/drivers/staging/rspiusb/rspiusb.c b/drivers/staging/rspiusb/rspiusb.c index ebdbe41fbcc3..1cdfe69585ea 100644 --- a/drivers/staging/rspiusb/rspiusb.c +++ b/drivers/staging/rspiusb/rspiusb.c @@ -217,8 +217,10 @@ static int pixis_io(struct ioctl_struct *ctrl, struct device_extension *pdx, dbg("numbytes to read = %d", numbytes); dbg("endpoint # %d", ctrl->endpoint); - if (copy_from_user(uBuf, ctrl->pData, numbytes)) + if (copy_from_user(uBuf, ctrl->pData, numbytes)) { dbg("copying ctrl->pData to dummyBuf failed"); + return -EFAULT; + } do { i = usb_bulk_msg(pdx->udev, pdx->hEP[ctrl->endpoint], @@ -304,9 +306,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, } switch (cmd) { case PIUSB_GETVNDCMD: - if (copy_from_user - (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) + if (__copy_from_user + (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) { dev_err(&pdx->udev->dev, "copy_from_user failed\n"); + return -EFAULT; + } dbg("%s %x\n", "Get Vendor Command = ", ctrl.cmd); retval = usb_control_msg(pdx->udev, usb_rcvctrlpipe(pdx->udev, 0), @@ -321,9 +325,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, return retval; case PIUSB_SETVNDCMD: - if (copy_from_user - (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) + if (__copy_from_user + (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) { dev_err(&pdx->udev->dev, "copy_from_user failed\n"); + return -EFAULT; + } /* dbg( "%s %x", "Set Vendor Command = ",ctrl.cmd ); */ controlData = ctrl.pData[0]; controlData |= (ctrl.pData[1] << 8); @@ -341,9 +347,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, return ((pdx->udev->speed == USB_SPEED_HIGH) ? 1 : 0); case PIUSB_WRITEPIPE: - if (copy_from_user(&ctrl, (void __user *)arg, _IOC_SIZE(cmd))) + if (__copy_from_user(&ctrl, (void __user *)arg, _IOC_SIZE(cmd))) { dev_err(&pdx->udev->dev, "copy_from_user WRITE_DUMMY failed\n"); + return -EFAULT; + } if (!access_ok(VERIFY_READ, ctrl.pData, ctrl.numbytes)) { dbg("can't access pData"); return 0; @@ -352,9 +360,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, return ctrl.numbytes; case PIUSB_USERBUFFER: - if (copy_from_user - (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) + if (__copy_from_user + (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) { dev_err(&pdx->udev->dev, "copy_from_user failed\n"); + return -EFAULT; + } return MapUserBuffer((struct ioctl_struct *) &ctrl, pdx); case PIUSB_UNMAP_USERBUFFER: @@ -362,10 +372,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, return retval; case PIUSB_READPIPE: - if (copy_from_user(&ctrl, (void __user *)arg, - sizeof(struct ioctl_struct))) + if (__copy_from_user(&ctrl, (void __user *)arg, + sizeof(struct ioctl_struct))) { dev_err(&pdx->udev->dev, "copy_from_user failed\n"); - + return -EFAULT; + } if (((0 == ctrl.endpoint) && (PIXIS_PID == pdx->iama)) || (1 == ctrl.endpoint) || /* ST133IO */ (4 == ctrl.endpoint)) /* PIXIS IO */ @@ -383,9 +394,11 @@ static int piusb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, case PIUSB_SETFRAMESIZE: dbg("PIUSB_SETFRAMESIZE"); - if (copy_from_user - (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) + if (__copy_from_user + (&ctrl, (void __user *)arg, sizeof(struct ioctl_struct))) { dev_err(&pdx->udev->dev, "copy_from_user failed\n"); + return -EFAULT; + } pdx->frameSize = ctrl.numbytes; pdx->num_frames = ctrl.numFrames; if (!pdx->sgl) @@ -451,7 +464,10 @@ int piusb_output(struct ioctl_struct *io, unsigned char *uBuf, int len, dev_err(&pdx->udev->dev, "buffer_alloc failed\n"); return -ENOMEM; } - memcpy(kbuf, uBuf, len); + if(__copy_from_user(kbuf, uBuf, len)) { + dev_err(&pdx->udev->dev, "__copy_from_user failed\n"); + return -EFAULT; + } usb_fill_bulk_urb(urb, pdx->udev, pdx->hEP[io->endpoint], kbuf, len, piusb_write_bulk_callback, pdx); urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; -- 2.30.2