From: Boris Brezillon <boris.brezil...@free-electrons.com>

Unlike what's done in mtd_read/write(), there are no checks to make sure
the parameters passed to mtd_read/write_oob() are consistent, which
forces implementers of ->_read/write_oob() to do it, which in turn leads
to code duplication and possibly errors in the logic.

Do general sanity checks, like ops fields consistency and range checking.

Signed-off-by: Boris Brezillon <boris.brezil...@free-electrons.com>
Cc: Peter Pan <peterpand...@micron.com>
Signed-off-by: Richard Weinberger <rich...@nod.at>
[Miquel: squashed the fix about the chip's size check]
Signed-off-by: Miquel Raynal <miquel.ray...@bootlin.com>
---
 drivers/mtd/mtdcore.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c22ca20368..ba170f212e 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1011,12 +1011,50 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, 
size_t len, size_t *retlen,
 }
 EXPORT_SYMBOL_GPL(mtd_panic_write);
 
+static int mtd_check_oob_ops(struct mtd_info *mtd, loff_t offs,
+                            struct mtd_oob_ops *ops)
+{
+       /*
+        * Some users are setting ->datbuf or ->oobbuf to NULL, but are leaving
+        * ->len or ->ooblen uninitialized. Force ->len and ->ooblen to 0 in
+        *  this case.
+        */
+       if (!ops->datbuf)
+               ops->len = 0;
+
+       if (!ops->oobbuf)
+               ops->ooblen = 0;
+
+       if (offs < 0 || offs + ops->len > mtd->size)
+               return -EINVAL;
+
+       if (ops->ooblen) {
+               u64 maxooblen;
+
+               if (ops->ooboffs >= mtd_oobavail(mtd, ops))
+                       return -EINVAL;
+
+               maxooblen = ((mtd_div_by_ws(mtd->size, mtd) -
+                             mtd_div_by_ws(offs, mtd)) *
+                            mtd_oobavail(mtd, ops)) - ops->ooboffs;
+               if (ops->ooblen > maxooblen)
+                       return -EINVAL;
+       }
+
+       return 0;
+}
+
 int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 {
        int ret_code;
        ops->retlen = ops->oobretlen = 0;
        if (!mtd->_read_oob)
                return -EOPNOTSUPP;
+
+       ret_code = mtd_check_oob_ops(mtd, from, ops);
+       if (ret_code)
+               return ret_code;
+
        /*
         * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
         * similar to mtd->_read(), returning a non-negative integer
@@ -1035,11 +1073,18 @@ EXPORT_SYMBOL_GPL(mtd_read_oob);
 int mtd_write_oob(struct mtd_info *mtd, loff_t to,
                                struct mtd_oob_ops *ops)
 {
+       int ret;
+
        ops->retlen = ops->oobretlen = 0;
        if (!mtd->_write_oob)
                return -EOPNOTSUPP;
        if (!(mtd->flags & MTD_WRITEABLE))
                return -EROFS;
+
+       ret = mtd_check_oob_ops(mtd, to, ops);
+       if (ret)
+               return ret;
+
        return mtd->_write_oob(mtd, to, ops);
 }
 EXPORT_SYMBOL_GPL(mtd_write_oob);
-- 
2.14.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to