summaryrefslogtreecommitdiff
path: root/drivers/mtd
diff options
context:
space:
mode:
authorMichał Kępień <kernel@kempniu.pl>2021-11-30 14:31:49 +0300
committerMiquel Raynal <miquel.raynal@bootlin.com>2021-12-09 19:52:29 +0300
commit6420ac0af95dbcb2fd8452e2d551ab50e1bbad83 (patch)
tree8c60fc35d095bb16a0daae2820a7813936363aea /drivers/mtd
parentdd8a2e884a462c09a562f04927cb227e3cdaa498 (diff)
downloadlinux-6420ac0af95dbcb2fd8452e2d551ab50e1bbad83.tar.xz
mtdchar: prevent unbounded allocation in MEMWRITE ioctl
In the mtdchar_write_ioctl() function, memdup_user() is called with its 'len' parameter set to verbatim values provided by user space via a struct mtd_write_req. Both the 'len' and 'ooblen' fields of that structure are 64-bit unsigned integers, which means the MEMWRITE ioctl can trigger unbounded kernel memory allocation requests. Fix by iterating over the buffers provided by user space in a loop, processing at most mtd->erasesize bytes in each iteration. Adopt some checks from mtd_check_oob_ops() to retain backward user space compatibility. Suggested-by: Boris Brezillon <boris.brezillon@collabora.com> Signed-off-by: Michał Kępień <kernel@kempniu.pl> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Link: https://lore.kernel.org/linux-mtd/20211130113149.21848-1-kernel@kempniu.pl
Diffstat (limited to 'drivers/mtd')
-rw-r--r--drivers/mtd/mtdchar.c110
1 files changed, 87 insertions, 23 deletions
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 155e991d9d75..d0f9c4b0285c 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -573,14 +573,32 @@ static int mtdchar_blkpg_ioctl(struct mtd_info *mtd,
}
}
+static void adjust_oob_length(struct mtd_info *mtd, uint64_t start,
+ struct mtd_oob_ops *ops)
+{
+ uint32_t start_page, end_page;
+ u32 oob_per_page;
+
+ if (ops->len == 0 || ops->ooblen == 0)
+ return;
+
+ start_page = mtd_div_by_ws(start, mtd);
+ end_page = mtd_div_by_ws(start + ops->len - 1, mtd);
+ oob_per_page = mtd_oobavail(mtd, ops);
+
+ ops->ooblen = min_t(size_t, ops->ooblen,
+ (end_page - start_page + 1) * oob_per_page);
+}
+
static int mtdchar_write_ioctl(struct mtd_info *mtd,
struct mtd_write_req __user *argp)
{
struct mtd_info *master = mtd_get_master(mtd);
struct mtd_write_req req;
- struct mtd_oob_ops ops = {};
const void __user *usr_data, *usr_oob;
- int ret;
+ uint8_t *datbuf = NULL, *oobbuf = NULL;
+ size_t datbuf_len, oobbuf_len;
+ int ret = 0;
if (copy_from_user(&req, argp, sizeof(req)))
return -EFAULT;
@@ -590,33 +608,79 @@ static int mtdchar_write_ioctl(struct mtd_info *mtd,
if (!master->_write_oob)
return -EOPNOTSUPP;
- ops.mode = req.mode;
- ops.len = (size_t)req.len;
- ops.ooblen = (size_t)req.ooblen;
- ops.ooboffs = 0;
-
- if (usr_data) {
- ops.datbuf = memdup_user(usr_data, ops.len);
- if (IS_ERR(ops.datbuf))
- return PTR_ERR(ops.datbuf);
- } else {
- ops.datbuf = NULL;
+
+ if (!usr_data)
+ req.len = 0;
+
+ if (!usr_oob)
+ req.ooblen = 0;
+
+ if (req.start + req.len > mtd->size)
+ return -EINVAL;
+
+ datbuf_len = min_t(size_t, req.len, mtd->erasesize);
+ if (datbuf_len > 0) {
+ datbuf = kmalloc(datbuf_len, GFP_KERNEL);
+ if (!datbuf)
+ return -ENOMEM;
}
- if (usr_oob) {
- ops.oobbuf = memdup_user(usr_oob, ops.ooblen);
- if (IS_ERR(ops.oobbuf)) {
- kfree(ops.datbuf);
- return PTR_ERR(ops.oobbuf);
+ oobbuf_len = min_t(size_t, req.ooblen, mtd->erasesize);
+ if (oobbuf_len > 0) {
+ oobbuf = kmalloc(oobbuf_len, GFP_KERNEL);
+ if (!oobbuf) {
+ kfree(datbuf);
+ return -ENOMEM;
}
- } else {
- ops.oobbuf = NULL;
}
- ret = mtd_write_oob(mtd, (loff_t)req.start, &ops);
+ while (req.len > 0 || (!usr_data && req.ooblen > 0)) {
+ struct mtd_oob_ops ops = {
+ .mode = req.mode,
+ .len = min_t(size_t, req.len, datbuf_len),
+ .ooblen = min_t(size_t, req.ooblen, oobbuf_len),
+ .datbuf = datbuf,
+ .oobbuf = oobbuf,
+ };
- kfree(ops.datbuf);
- kfree(ops.oobbuf);
+ /*
+ * Shorten non-page-aligned, eraseblock-sized writes so that
+ * the write ends on an eraseblock boundary. This is necessary
+ * for adjust_oob_length() to properly handle non-page-aligned
+ * writes.
+ */
+ if (ops.len == mtd->erasesize)
+ ops.len -= mtd_mod_by_ws(req.start + ops.len, mtd);
+
+ /*
+ * For writes which are not OOB-only, adjust the amount of OOB
+ * data written according to the number of data pages written.
+ * This is necessary to prevent OOB data from being skipped
+ * over in data+OOB writes requiring multiple mtd_write_oob()
+ * calls to be completed.
+ */
+ adjust_oob_length(mtd, req.start, &ops);
+
+ if (copy_from_user(datbuf, usr_data, ops.len) ||
+ copy_from_user(oobbuf, usr_oob, ops.ooblen)) {
+ ret = -EFAULT;
+ break;
+ }
+
+ ret = mtd_write_oob(mtd, req.start, &ops);
+ if (ret)
+ break;
+
+ req.start += ops.retlen;
+ req.len -= ops.retlen;
+ usr_data += ops.retlen;
+
+ req.ooblen -= ops.oobretlen;
+ usr_oob += ops.oobretlen;
+ }
+
+ kfree(datbuf);
+ kfree(oobbuf);
return ret;
}