All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kulikov Vasiliy <segooon@gmail.com>
To: kernel-janitors@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	H Hartley Sweeten <hsweeten@visionengravers.com>,
	Simon Horman <horms@verge.net.au>, Joe Eloff <kagen101@gmail.com>,
	Randy Dunlap <randy.dunlap@oracle.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/9] staging: dt3155: check put_user() return value
Date: Fri, 30 Jul 2010 11:07:09 +0000	[thread overview]
Message-ID: <1280488030-20697-1-git-send-email-segooon@gmail.com> (raw)

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@gmail.com>
---
 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


WARNING: multiple messages have this Message-ID (diff)
From: Kulikov Vasiliy <segooon@gmail.com>
To: kernel-janitors@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	H Hartley Sweeten <hsweeten@visionengravers.com>,
	Simon Horman <horms@verge.net.au>, Joe Eloff <kagen101@gmail.com>,
	Randy Dunlap <randy.dunlap@oracle.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/9] staging: dt3155: check put_user() return value
Date: Fri, 30 Jul 2010 15:07:09 +0400	[thread overview]
Message-ID: <1280488030-20697-1-git-send-email-segooon@gmail.com> (raw)

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@gmail.com>
---
 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


             reply	other threads:[~2010-07-30 11:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-30 11:07 Kulikov Vasiliy [this message]
2010-07-30 11:07 ` [PATCH 1/9] staging: dt3155: check put_user() return value Kulikov Vasiliy
2010-07-30 12:44 ` Jiri Slaby
2010-07-30 12:44   ` Jiri Slaby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1280488030-20697-1-git-send-email-segooon@gmail.com \
    --to=segooon@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@suse.de \
    --cc=horms@verge.net.au \
    --cc=hsweeten@visionengravers.com \
    --cc=kagen101@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=randy.dunlap@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.