[PATCH 1/9] staging: dt3155: check put_user() return value

[Kulikov Vasiliy]

put_user() may fail, if so return -EFAULT.
Also compare count with copied data size, not size of struct with these
fields.

Signed-off-by: Kulikov Vasiliy <segooon@xxxxxxxxx>
---
 drivers/staging/dt3155/dt3155_drv.c |  121 ++++++++++++++++------------------
 1 files changed, 57 insertions(+), 64 deletions(-)

diff --git a/drivers/staging/dt3155/dt3155_drv.c b/drivers/staging/dt3155/dt3155_drv.c
index fed7e62..cfe4fae 100644
--- a/drivers/staging/dt3155/dt3155_drv.c
+++ b/drivers/staging/dt3155/dt3155_drv.c
@@ -737,77 +737,70 @@ static int dt3155_close(struct inode *inode, struct file *filep)
 static ssize_t dt3155_read(struct file *filep, char __user *buf,
 			   size_t count, loff_t *ppos)
 {
-  /* which device are we reading from? */
-  int		minor = MINOR(filep->f_dentry->d_inode->i_rdev);
-  u32		offset;
-  int		frame_index;
-  struct dt3155_status *dts = &dt3155_status[minor];
-  struct dt3155_fbuffer *fb = &dts->fbuffer;
-  struct frame_info	*frame_info;
-
-  /* TODO: this should check the error flag and */
-  /*   return an error on hardware failures */
-  if (count != sizeof(struct dt3155_read))
-    {
-      printk("DT3155 ERROR (NJC): count is not right\n");
-      return -EINVAL;
-    }
-
-
-  /* Hack here -- I'm going to allow reading even when idle.
-   * this is so that the frames can be read after STOP has
-   * been called.  Leaving it here, commented out, as a reminder
-   * for a short while to make sure there are no problems.
-   * Note that if the driver is not opened in non_blocking mode,
-   * and the device is idle, then it could sit here forever! */
+	/* which device are we reading from? */
+	int minor = MINOR(filep->f_dentry->d_inode->i_rdev);
+	u32 offset;
+	int frame_index;
+	struct dt3155_status *dts = &dt3155_status[minor];
+	struct dt3155_fbuffer *fb = &dts->fbuffer;
+	struct frame_info *frame_info;
+	u32 *buffer = (u32 *)buf;
+
+	/* TODO: this should check the error flag and */
+	/*   return an error on hardware failures */
+	if (count != 4*3 + sizeof(struct frame_info)) {
+		pr_err("DT3155 ERROR (NJC): count is not right\n");
+		return -EINVAL;
+	}
 
-  /*  if (dts->state == DT3155_STATE_IDLE)*/
-  /*    return -EBUSY;*/
 
-  /* non-blocking reads should return if no data */
-  if (filep->f_flags & O_NDELAY)
-    {
-      if ((frame_index = dt3155_get_ready_buffer(minor)) < 0) {
-	/*printk("dt3155:  no buffers available (?)\n");*/
-	/* 		printques(minor); */
-	return -EAGAIN;
-      }
-    }
-  else
-    {
-      /*
-       * sleep till data arrives , or we get interrupted.
-       * Note that wait_event_interruptible() does not actually
-       * sleep/wait if it's condition evaluates to true upon entry.
-       */
-      wait_event_interruptible(dt3155_read_wait_queue[minor],
-			       (frame_index = dt3155_get_ready_buffer(minor))
-			       >= 0);
-
-      if (frame_index < 0)
-	{
-	  printk ("DT3155: read: interrupted\n");
-	  quick_stop (minor);
-	  printques(minor);
-	  return -EINTR;
+	/* Hack here -- I'm going to allow reading even when idle.
+	* this is so that the frames can be read after STOP has
+	* been called.  Leaving it here, commented out, as a reminder
+	* for a short while to make sure there are no problems.
+	* Note that if the driver is not opened in non_blocking mode,
+	* and the device is idle, then it could sit here forever! */
+
+	/*  if (dts->state == DT3155_STATE_IDLE)*/
+	/*    return -EBUSY;*/
+
+	/* non-blocking reads should return if no data */
+	if (filep->f_flags & O_NDELAY) {
+		frame_index = dt3155_get_ready_buffer(minor);
+		if (frame_index < 0)
+			/*printk("dt3155:  no buffers available (?)\n");*/
+			/* printques(minor); */
+			return -EAGAIN;
+	} else {
+		/*
+		* sleep till data arrives , or we get interrupted.
+		* Note that wait_event_interruptible() does not actually
+		* sleep/wait if it's condition evaluates to true upon entry.
+		*/
+		wait_event_interruptible(dt3155_read_wait_queue[minor],
+				  (frame_index = dt3155_get_ready_buffer(minor))
+				  >= 0);
+
+		if (frame_index < 0) {
+			pr_info("DT3155: read: interrupted\n");
+			quick_stop(minor);
+			printques(minor);
+			return -EINTR;
+		}
 	}
-    }
 
-  frame_info = &fb->frame_info[frame_index];
+	frame_info = &fb->frame_info[frame_index];
 
-  /* make this an offset */
-  offset = frame_info->addr - dts->mem_addr;
+	/* make this an offset */
+	offset = frame_info->addr - dts->mem_addr;
 
-  put_user(offset, (unsigned int __user *)buf);
-  buf += sizeof(u32);
-  put_user(fb->frame_count, (unsigned int __user *)buf);
-  buf += sizeof(u32);
-  put_user(dts->state, (unsigned int __user *)buf);
-  buf += sizeof(u32);
-  if (copy_to_user(buf, frame_info, sizeof(*frame_info)))
-      return -EFAULT;
+	if (put_user(offset, buffer) ||
+	    put_user(fb->frame_count, buffer + 1) ||
+	    put_user(dts->state, buffer + 2) ||
+	    copy_to_user(buffer + 3, frame_info, sizeof(*frame_info)))
+		return -EFAULT;
 
-  return sizeof(struct dt3155_read);
+	return count;
 }
 
 static unsigned int dt3155_poll (struct file * filp, poll_table *wait)
-- 
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/