All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
To: linux-block@vger.kernel.org
Cc: Chaitanya.Kulkarni@wdc.com, bvanassche@acm.org,
	Harshad Shirwadkar <harshadshirwadkar@gmail.com>
Subject: [PATCH v2] blktrace: put bounds on BLKTRACESETUP buf_size and buf_nr
Date: Wed, 10 Jun 2020 18:33:26 -0700	[thread overview]
Message-ID: <20200611013326.218542-1-harshadshirwadkar@gmail.com> (raw)

Make sure that user requested memory via BLKTRACESETUP is within
bounds. This can be easily exploited by setting really large values
for buf_size and buf_nr in BLKTRACESETUP ioctl.

blktrace program has following hardcoded values for bufsize and bufnr:
BUF_SIZE=(512 * 1024)
BUF_NR=(4)

This is very easy to exploit. Setting buf_size / buf_nr in userspace
program to big values make kernel go oom.

This patch adds a new new sysfs tunable called "blktrace_max_alloc"
with the default value as:
blktrace_max_alloc=(1024 * 1024 * 16)

Verified that the fix makes BLKTRACESETUP return -E2BIG if the
buf_size * buf_nr crosses the configured upper bound in the device's
sysfs file. Verified that the bound checking is turned off when we
write 0 to the sysfs file.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 Documentation/block/queue-sysfs.rst |  8 ++++++++
 block/blk-settings.c                |  1 +
 block/blk-sysfs.c                   | 27 +++++++++++++++++++++++++++
 include/linux/blkdev.h              |  5 +++++
 kernel/trace/blktrace.c             |  8 ++++++++
 5 files changed, 49 insertions(+)

diff --git a/Documentation/block/queue-sysfs.rst b/Documentation/block/queue-sysfs.rst
index 6a8513af9201..ef4eec68cd24 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -251,4 +251,12 @@ devices are described in the ZBC (Zoned Block Commands) and ZAC
 do not support zone commands, they will be treated as regular block devices
 and zoned will report "none".
 
+blktrace_max_alloc (RW)
+----------
+BLKTRACESETUP ioctl takes the number of buffers and the size of each
+buffer as an argument and uses these values to allocate kernel memory
+for tracing purpose. This is the limit on the maximum kernel memory
+that can be allocated by BLKTRACESETUP ioctl. The default limit is
+16MB. Value of 0 indicates that this bound checking is disabled.
+
 Jens Axboe <jens.axboe@oracle.com>, February 2009
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 9a2c23cd9700..38535aa146f4 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -60,6 +60,7 @@ void blk_set_default_limits(struct queue_limits *lim)
 	lim->io_opt = 0;
 	lim->misaligned = 0;
 	lim->zoned = BLK_ZONED_NONE;
+	lim->blktrace_max_alloc = BLKTRACE_MAX_ALLOC;
 }
 EXPORT_SYMBOL(blk_set_default_limits);
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 02643e149d5e..e849e80930c4 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -496,6 +496,26 @@ static ssize_t queue_wb_lat_store(struct request_queue *q, const char *page,
 	return count;
 }
 
+static ssize_t queue_blktrace_max_alloc_show(struct request_queue *q, char *page)
+{
+	return queue_var_show(q->limits.blktrace_max_alloc, page);
+}
+
+static ssize_t queue_blktrace_max_alloc_store(struct request_queue *q,
+					      const char *page,
+					      size_t count)
+{
+	unsigned long blktrace_max_alloc;
+	int ret;
+
+	ret = queue_var_store(&blktrace_max_alloc, page, count);
+	if (ret < 0)
+		return ret;
+
+	q->limits.blktrace_max_alloc = blktrace_max_alloc;
+	return count;
+}
+
 static ssize_t queue_wc_show(struct request_queue *q, char *page)
 {
 	if (test_bit(QUEUE_FLAG_WC, &q->queue_flags))
@@ -731,6 +751,12 @@ static struct queue_sysfs_entry queue_wb_lat_entry = {
 	.store = queue_wb_lat_store,
 };
 
+static struct queue_sysfs_entry queue_blktrace_max_alloc_entry = {
+	.attr = {.name = "blktrace_max_alloc", .mode = 0644 },
+	.show = queue_blktrace_max_alloc_show,
+	.store = queue_blktrace_max_alloc_store,
+};
+
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 static struct queue_sysfs_entry throtl_sample_time_entry = {
 	.attr = {.name = "throttle_sample_time", .mode = 0644 },
@@ -779,6 +805,7 @@ static struct attribute *queue_attrs[] = {
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 	&throtl_sample_time_entry.attr,
 #endif
+	&queue_blktrace_max_alloc_entry.attr,
 	NULL,
 };
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8fd900998b4e..3a72b0fd723e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -309,6 +309,10 @@ enum blk_queue_state {
 #define BLK_SCSI_MAX_CMDS	(256)
 #define BLK_SCSI_CMD_PER_LONG	(BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
 
+#define BLKTRACE_MAX_BUFSIZ	(1024 * 1024)
+#define BLKTRACE_MAX_BUFNR	16
+#define BLKTRACE_MAX_ALLOC	((BLKTRACE_MAX_BUFSIZ) * (BLKTRACE_MAX_BUFNR))
+
 /*
  * Zoned block device models (zoned limit).
  */
@@ -322,6 +326,7 @@ struct queue_limits {
 	unsigned long		bounce_pfn;
 	unsigned long		seg_boundary_mask;
 	unsigned long		virt_boundary_mask;
+	unsigned long		blktrace_max_alloc;
 
 	unsigned int		max_hw_sectors;
 	unsigned int		max_dev_sectors;
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index ea47f2084087..de028bdbb148 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -477,11 +477,19 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 {
 	struct blk_trace *bt = NULL;
 	struct dentry *dir = NULL;
+	u32 alloc_siz;
 	int ret;
 
 	if (!buts->buf_size || !buts->buf_nr)
 		return -EINVAL;
 
+	if (check_mul_overflow(buts->buf_size, buts->buf_nr, &alloc_siz))
+		return -E2BIG;
+
+	if (q->limits.blktrace_max_alloc &&
+	    alloc_siz > q->limits.blktrace_max_alloc)
+		return -E2BIG;
+
 	if (!blk_debugfs_root)
 		return -ENOENT;
 
-- 
2.27.0.278.ge193c7cf3a9-goog


             reply	other threads:[~2020-06-11  1:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-11  1:33 Harshad Shirwadkar [this message]
2020-06-11  1:53 ` [PATCH v2] blktrace: put bounds on BLKTRACESETUP buf_size and buf_nr Chaitanya Kulkarni
2020-06-11 13:41 ` Bart Van Assche
2020-06-13  0:09   ` Chaitanya Kulkarni
2020-06-13  0:57     ` harshad shirwadkar
2020-06-15 11:52 ` Christoph Hellwig
2020-06-15 14:01   ` Bart Van Assche
2020-06-15 14:04     ` Christoph Hellwig

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=20200611013326.218542-1-harshadshirwadkar@gmail.com \
    --to=harshadshirwadkar@gmail.com \
    --cc=Chaitanya.Kulkarni@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=linux-block@vger.kernel.org \
    /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.