diff mbox

[v2,03/12] blk-mq-debugfs: error on long write to queue "state" file

Message ID 859ef1939e07171f902b4fbb5bebbb274d68df18.1493882751.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show

Commit Message

Omar Sandoval May 4, 2017, 7:31 a.m. UTC
From: Omar Sandoval <osandov@fb.com>

blk_queue_flags_store() currently truncates and returns a short write if
the operation being written is too long. This can give us weird results,
like here:

$ echo "run            bar"
echo: write error: invalid argument
$ dmesg
[ 1103.075435] blk_queue_flags_store: unsupported operation bar. Use either 'run' or 'start'

Instead, return an error if the user does this. While we're here, make
the argument names consistent with everywhere else in this file.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 block/blk-mq-debugfs.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Hannes Reinecke May 4, 2017, 11:21 a.m. UTC | #1
On 05/04/2017 09:31 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> blk_queue_flags_store() currently truncates and returns a short write if
> the operation being written is too long. This can give us weird results,
> like here:
> 
> $ echo "run            bar"
> echo: write error: invalid argument
> $ dmesg
> [ 1103.075435] blk_queue_flags_store: unsupported operation bar. Use either 'run' or 'start'
> 
> Instead, return an error if the user does this. While we're here, make
> the argument names consistent with everywhere else in this file.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  block/blk-mq-debugfs.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
Bart Van Assche May 4, 2017, 11:06 p.m. UTC | #2
On Thu, 2017-05-04 at 00:31 -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> blk_queue_flags_store() currently truncates and returns a short write if
> the operation being written is too long. This can give us weird results,
> like here:
> 
> $ echo "run            bar"
> echo: write error: invalid argument
> $ dmesg
> [ 1103.075435] blk_queue_flags_store: unsupported operation bar. Use either 'run' or 'start'
> 
> Instead, return an error if the user does this. While we're here, make
> the argument names consistent with everywhere else in this file.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
diff mbox

Patch

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index f58a116d6cca..2a19237455d4 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -107,14 +107,18 @@  static int blk_queue_flags_show(struct seq_file *m, void *v)
 	return 0;
 }
 
-static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
-				     size_t len, loff_t *offp)
+static ssize_t blk_queue_flags_store(struct file *file, const char __user *buf,
+				     size_t count, loff_t *ppos)
 {
 	struct request_queue *q = file_inode(file)->i_private;
 	char op[16] = { }, *s;
 
-	len = min(len, sizeof(op) - 1);
-	if (copy_from_user(op, ubuf, len))
+	if (count >= sizeof(op)) {
+		pr_err("%s: operation too long\n", __func__);
+		goto inval;
+	}
+
+	if (copy_from_user(op, buf, count))
 		return -EFAULT;
 	s = op;
 	strsep(&s, " \t\n"); /* strip trailing whitespace */
@@ -123,11 +127,12 @@  static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
 	} else if (strcmp(op, "start") == 0) {
 		blk_mq_start_stopped_hw_queues(q, true);
 	} else {
-		pr_err("%s: unsupported operation %s. Use either 'run' or 'start'\n",
-		       __func__, op);
+		pr_err("%s: unsupported operation '%s'\n", __func__, op);
+inval:
+		pr_err("%s: use either 'run' or 'start'\n", __func__);
 		return -EINVAL;
 	}
-	return len;
+	return count;
 }
 
 static int blk_queue_flags_open(struct inode *inode, struct file *file)