* [PATCH 1/4] mtd: return error code from mtd_unpoint
@ 2012-02-03 17:01 Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 2/4] mtd: add offset and length checks to the API function Artem Bityutskiy
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-02-03 17:01 UTC (permalink / raw)
To: MTD Maling List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
The 'mtd_unpoint()' API function should be able to return an error code because
it may fail if you specify incorrect offset. This patch changes this MTD API
function and amends all the drivers correspondingly.
Also return '-EOPNOTSUPP' from 'mtd_unpoint()' when the '->unpoint()' method is
undefined. We do not really need this currently, but this just makes
sense to be consistent with 'mtd_point()'.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
drivers/mtd/chips/cfi_cmdset_0001.c | 16 ++++++++++------
drivers/mtd/devices/mtdram.c | 3 ++-
drivers/mtd/devices/phram.c | 3 ++-
drivers/mtd/devices/pmc551.c | 2 +-
drivers/mtd/devices/slram.c | 5 +++--
drivers/mtd/lpddr/lpddr_cmds.c | 12 ++++++++----
drivers/mtd/mtdpart.c | 4 ++--
include/linux/mtd/mtd.h | 6 ++++--
8 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 152accf..4d04551 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -87,7 +87,7 @@ static int cfi_intelext_partition_fixup(struct mtd_info *, struct cfi_private **
static int cfi_intelext_point (struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys);
-static void cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
+static int cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
static int chip_ready (struct map_info *map, struct flchip *chip, unsigned long adr, int mode);
static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode);
@@ -1369,12 +1369,12 @@ static int cfi_intelext_point(struct mtd_info *mtd, loff_t from, size_t len,
return 0;
}
-static void cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
struct map_info *map = mtd->priv;
struct cfi_private *cfi = map->fldrv_priv;
unsigned long ofs;
- int chipnum;
+ int chipnum, err = 0;
/* Now unlock the chip(s) POINT state */
@@ -1382,7 +1382,7 @@ static void cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
chipnum = (from >> cfi->chipshift);
ofs = from - (chipnum << cfi->chipshift);
- while (len) {
+ while (len && !err) {
unsigned long thislen;
struct flchip *chip;
@@ -1400,8 +1400,10 @@ static void cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
chip->ref_point_counter--;
if(chip->ref_point_counter == 0)
chip->state = FL_READY;
- } else
- printk(KERN_ERR "%s: Warning: unpoint called on non pointed region\n", map->name); /* Should this give an error? */
+ } else {
+ printk(KERN_ERR "%s: Error: unpoint called on non pointed region\n", map->name);
+ err = -EINVAL;
+ }
put_chip(map, chip, chip->start);
mutex_unlock(&chip->mutex);
@@ -1410,6 +1412,8 @@ static void cfi_intelext_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
ofs = 0;
chipnum++;
}
+
+ return err;
}
static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 91030cf..e1f017b 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -60,8 +60,9 @@ static int ram_point(struct mtd_info *mtd, loff_t from, size_t len,
return 0;
}
-static void ram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int ram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
+ return 0;
}
/*
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index eff2b69..3803555 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -70,8 +70,9 @@ static int phram_point(struct mtd_info *mtd, loff_t from, size_t len,
return 0;
}
-static void phram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int phram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
+ return 0;
}
static int phram_read(struct mtd_info *mtd, loff_t from, size_t len,
diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
index 67d22e1..40a96c7 100644
--- a/drivers/mtd/devices/pmc551.c
+++ b/drivers/mtd/devices/pmc551.c
@@ -206,7 +206,7 @@ static int pmc551_point(struct mtd_info *mtd, loff_t from, size_t len,
return 0;
}
-static void pmc551_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int pmc551_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
#ifdef CONFIG_MTD_PMC551_DEBUG
printk(KERN_DEBUG "pmc551_unpoint()\n");
diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
index cbeb195..9431ffc 100644
--- a/drivers/mtd/devices/slram.c
+++ b/drivers/mtd/devices/slram.c
@@ -76,7 +76,7 @@ static slram_mtd_list_t *slram_mtdlist = NULL;
static int slram_erase(struct mtd_info *, struct erase_info *);
static int slram_point(struct mtd_info *, loff_t, size_t, size_t *, void **,
resource_size_t *);
-static void slram_unpoint(struct mtd_info *, loff_t, size_t);
+static int slram_unpoint(struct mtd_info *, loff_t, size_t);
static int slram_read(struct mtd_info *, loff_t, size_t, size_t *, u_char *);
static int slram_write(struct mtd_info *, loff_t, size_t, size_t *, const u_char *);
@@ -119,8 +119,9 @@ static int slram_point(struct mtd_info *mtd, loff_t from, size_t len,
return(0);
}
-static void slram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int slram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
+ return 0;
}
static int slram_read(struct mtd_info *mtd, loff_t from, size_t len,
diff --git a/drivers/mtd/lpddr/lpddr_cmds.c b/drivers/mtd/lpddr/lpddr_cmds.c
index fd19d3b..de960b1 100644
--- a/drivers/mtd/lpddr/lpddr_cmds.c
+++ b/drivers/mtd/lpddr/lpddr_cmds.c
@@ -40,7 +40,7 @@ static int lpddr_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
static int lpddr_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
static int lpddr_point(struct mtd_info *mtd, loff_t adr, size_t len,
size_t *retlen, void **mtdbuf, resource_size_t *phys);
-static void lpddr_unpoint(struct mtd_info *mtd, loff_t adr, size_t len);
+static int lpddr_unpoint(struct mtd_info *mtd, loff_t adr, size_t len);
static int get_chip(struct map_info *map, struct flchip *chip, int mode);
static int chip_ready(struct map_info *map, struct flchip *chip, int mode);
static void put_chip(struct map_info *map, struct flchip *chip);
@@ -575,11 +575,11 @@ static int lpddr_point(struct mtd_info *mtd, loff_t adr, size_t len,
return 0;
}
-static void lpddr_unpoint (struct mtd_info *mtd, loff_t adr, size_t len)
+static int lpddr_unpoint (struct mtd_info *mtd, loff_t adr, size_t len)
{
struct map_info *map = mtd->priv;
struct lpddr_private *lpddr = map->fldrv_priv;
- int chipnum = adr >> lpddr->chipshift;
+ int chipnum = adr >> lpddr->chipshift, err = 0;
unsigned long ofs;
/* ofs: offset within the first chip that the first read should start */
@@ -603,9 +603,11 @@ static void lpddr_unpoint (struct mtd_info *mtd, loff_t adr, size_t len)
chip->ref_point_counter--;
if (chip->ref_point_counter == 0)
chip->state = FL_READY;
- } else
+ } else {
printk(KERN_WARNING "%s: Warning: unpoint called on non"
"pointed region\n", map->name);
+ err = -EINVAL;
+ }
put_chip(map, chip);
mutex_unlock(&chip->mutex);
@@ -614,6 +616,8 @@ static void lpddr_unpoint (struct mtd_info *mtd, loff_t adr, size_t len)
ofs = 0;
chipnum++;
}
+
+ return err;
}
static int lpddr_write_buffers(struct mtd_info *mtd, loff_t to, size_t len,
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 4f01079..da8a0b2 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -92,11 +92,11 @@ static int part_point(struct mtd_info *mtd, loff_t from, size_t len,
virt, phys);
}
-static void part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
struct mtd_part *part = PART(mtd);
- mtd_unpoint(part->master, from + part->offset, len);
+ return mtd_unpoint(part->master, from + part->offset, len);
}
static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e2e5456..8c24311 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -177,7 +177,7 @@ struct mtd_info {
int (*_erase) (struct mtd_info *mtd, struct erase_info *instr);
int (*_point) (struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys);
- void (*_unpoint) (struct mtd_info *mtd, loff_t from, size_t len);
+ int (*_unpoint) (struct mtd_info *mtd, loff_t from, size_t len);
unsigned long (*_get_unmapped_area) (struct mtd_info *mtd,
unsigned long len,
unsigned long offset,
@@ -265,8 +265,10 @@ static inline int mtd_point(struct mtd_info *mtd, loff_t from, size_t len,
}
/* We probably shouldn't allow XIP if the unpoint isn't a NULL */
-static inline void mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+static inline int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
+ if (!mtd->_point)
+ return -EOPNOTSUPP;
return mtd->_unpoint(mtd, from, len);
}
--
1.7.9
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] mtd: add offset and length checks to the API function
2012-02-03 17:01 [PATCH 1/4] mtd: return error code from mtd_unpoint Artem Bityutskiy
@ 2012-02-03 17:01 ` Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 3/4] mtd: do not duplicate length and offset checks in drivers Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 4/4] mtd: remove R/O checking duplication Artem Bityutskiy
2 siblings, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-02-03 17:01 UTC (permalink / raw)
To: MTD Maling List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Add verification of the offset and length to MTD API functions and verify that
MTD device offset and length are within MTD device size.
The modified API functions are:
'mtd_erase()'
'mtd_point()'
'mtd_unpoint()'
'mtd_get_unmapped_area()'
'mtd_read()'
'mtd_write()'
'mtd_panic_write()'
'mtd_lock()'
'mtd_unlock()'
'mtd_is_locked()'
'mtd_block_isbad()'
'mtd_block_markbad()'
This patch also uninlines these functions and exports in mtdcore.c because they
are not performance-critical and do not have to be inlined.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
drivers/mtd/mtdcore.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 129 +++++-----------------------------------
2 files changed, 167 insertions(+), 112 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5ea22cf..13bd4fa 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -683,6 +683,156 @@ void __put_mtd_device(struct mtd_info *mtd)
EXPORT_SYMBOL_GPL(__put_mtd_device);
/*
+ * Erase is an asynchronous operation. Device drivers are supposed
+ * to call instr->callback() whenever the operation completes, even
+ * if it completes with a failure.
+ * Callers are supposed to pass a callback function and wait for it
+ * to be called before writing to the block.
+ */
+int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+ if (instr->addr > mtd->size || instr->len > mtd->size - instr->addr)
+ return -EINVAL;
+ return mtd->_erase(mtd, instr);
+}
+EXPORT_SYMBOL_GPL(mtd_erase);
+
+/*
+ * This stuff for eXecute-In-Place. phys is optional and may be set to NULL.
+ */
+int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
+ void **virt, resource_size_t *phys)
+{
+ uint64_t ofs = from;
+
+ *retlen = 0;
+ if (!mtd->_point)
+ return -EOPNOTSUPP;
+ if (from < 0 || from > mtd->size || len > mtd->size - from)
+ return -EINVAL;
+ return mtd->_point(mtd, from, len, retlen, virt, phys);
+}
+EXPORT_SYMBOL_GPL(mtd_point);
+
+/* We probably shouldn't allow XIP if the unpoint isn't a NULL */
+int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+{
+ if (!mtd->_point)
+ return -EOPNOTSUPP;
+ if (from < 0 || from > mtd->size || len > mtd->size - from)
+ return -EINVAL;
+ return mtd->_unpoint(mtd, from, len);
+}
+EXPORT_SYMBOL_GPL(mtd_unpoint);
+
+/*
+ * Allow NOMMU mmap() to directly map the device (if not NULL)
+ * - return the address to which the offset maps
+ * - return -ENOSYS to indicate refusal to do the mapping
+ */
+unsigned long mtd_get_unmapped_area(struct mtd_info *mtd, unsigned long len,
+ unsigned long offset, unsigned long flags)
+{
+ if (!mtd->_get_unmapped_area)
+ return -EOPNOTSUPP;
+ if (offset > mtd->size || len > mtd->size - offset)
+ return -EINVAL;
+ return mtd->_get_unmapped_area(mtd, len, offset, flags);
+}
+EXPORT_SYMBOL_GPL(mtd_get_unmapped_area);
+
+int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
+ u_char *buf)
+{
+ if (from < 0 || from > mtd->size || len > mtd->size - from)
+ return -EINVAL;
+ return mtd->_read(mtd, from, len, retlen, buf);
+}
+EXPORT_SYMBOL_GPL(mtd_read);
+
+int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
+ const u_char *buf)
+{
+ *retlen = 0;
+ if (!mtd->_write)
+ return -EROFS;
+ if (to < 0 || to > mtd->size || len > mtd->size - to)
+ return -EINVAL;
+ return mtd->_write(mtd, to, len, retlen, buf);
+}
+EXPORT_SYMBOL_GPL(mtd_write);
+
+/*
+ * In blackbox flight recorder like scenarios we want to make successful writes
+ * in interrupt context. panic_write() is only intended to be called when its
+ * known the kernel is about to panic and we need the write to succeed. Since
+ * the kernel is not going to be running for much longer, this function can
+ * break locks and delay to ensure the write succeeds (but not sleep).
+ */
+int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
+ const u_char *buf)
+{
+ *retlen = 0;
+ if (!mtd->_panic_write)
+ return -EOPNOTSUPP;
+ if (to < 0 || to > mtd->size || len > mtd->size - to)
+ return -EINVAL;
+ return mtd->_panic_write(mtd, to, len, retlen, buf);
+}
+EXPORT_SYMBOL_GPL(mtd_panic_write);
+
+/* Chip-supported device locking */
+int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+ if (!mtd->_lock)
+ return -EOPNOTSUPP;
+ if (ofs < 0 || ofs > mtd->size || len > mtd->size - ofs)
+ return -EINVAL;
+ return mtd->_lock(mtd, ofs, len);
+}
+EXPORT_SYMBOL_GPL(mtd_lock);
+
+int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+ if (!mtd->_unlock)
+ return -EOPNOTSUPP;
+ if (ofs < 0 || ofs > mtd->size || len > mtd->size - ofs)
+ return -EINVAL;
+ return mtd->_unlock(mtd, ofs, len);
+}
+EXPORT_SYMBOL_GPL(mtd_unlock);
+
+int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+ if (!mtd->_is_locked)
+ return -EOPNOTSUPP;
+ if (ofs < 0 || ofs > mtd->size || len > mtd->size - ofs)
+ return -EINVAL;
+ return mtd->_is_locked(mtd, ofs, len);
+}
+EXPORT_SYMBOL_GPL(mtd_is_locked);
+
+int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
+{
+ if (!mtd->_block_isbad)
+ return 0;
+ if (ofs < 0 || ofs > mtd->size)
+ return -EINVAL;
+ return mtd->_block_isbad(mtd, ofs);
+}
+EXPORT_SYMBOL_GPL(mtd_block_isbad);
+
+int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+ if (!mtd->_block_markbad)
+ return -EOPNOTSUPP;
+ if (ofs < 0 || ofs > mtd->size)
+ return -EINVAL;
+ return mtd->_block_markbad(mtd, ofs);
+}
+EXPORT_SYMBOL_GPL(mtd_block_markbad);
+
+/*
* default_mtd_writev - the default writev method
* @mtd: mtd device description object pointer
* @vecs: the vectors to write
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 8c24311..317a80c 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -240,83 +240,18 @@ struct mtd_info {
int usecount;
};
-/*
- * Erase is an asynchronous operation. Device drivers are supposed
- * to call instr->callback() whenever the operation completes, even
- * if it completes with a failure.
- * Callers are supposed to pass a callback function and wait for it
- * to be called before writing to the block.
- */
-static inline int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
-{
- return mtd->_erase(mtd, instr);
-}
-
-/*
- * This stuff for eXecute-In-Place. phys is optional and may be set to NULL.
- */
-static inline int mtd_point(struct mtd_info *mtd, loff_t from, size_t len,
- size_t *retlen, void **virt, resource_size_t *phys)
-{
- *retlen = 0;
- if (!mtd->_point)
- return -EOPNOTSUPP;
- return mtd->_point(mtd, from, len, retlen, virt, phys);
-}
-
-/* We probably shouldn't allow XIP if the unpoint isn't a NULL */
-static inline int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
-{
- if (!mtd->_point)
- return -EOPNOTSUPP;
- return mtd->_unpoint(mtd, from, len);
-}
-
-/*
- * Allow NOMMU mmap() to directly map the device (if not NULL)
- * - return the address to which the offset maps
- * - return -ENOSYS to indicate refusal to do the mapping
- */
-static inline unsigned long mtd_get_unmapped_area(struct mtd_info *mtd,
- unsigned long len,
- unsigned long offset,
- unsigned long flags)
-{
- if (!mtd->_get_unmapped_area)
- return -EOPNOTSUPP;
- return mtd->_get_unmapped_area(mtd, len, offset, flags);
-}
-
-static inline int mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
- size_t *retlen, u_char *buf)
-{
- return mtd->_read(mtd, from, len, retlen, buf);
-}
-
-static inline int mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
-{
- *retlen = 0;
- if (!mtd->_write)
- return -EROFS;
- return mtd->_write(mtd, to, len, retlen, buf);
-}
-
-/*
- * In blackbox flight recorder like scenarios we want to make successful writes
- * in interrupt context. panic_write() is only intended to be called when its
- * known the kernel is about to panic and we need the write to succeed. Since
- * the kernel is not going to be running for much longer, this function can
- * break locks and delay to ensure the write succeeds (but not sleep).
- */
-static inline int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
- size_t *retlen, const u_char *buf)
-{
- *retlen = 0;
- if (!mtd->_panic_write)
- return -EOPNOTSUPP;
- return mtd->_panic_write(mtd, to, len, retlen, buf);
-}
+int mtd_erase(struct mtd_info *mtd, struct erase_info *instr);
+int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
+ void **virt, resource_size_t *phys);
+int mtd_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
+unsigned long mtd_get_unmapped_area(struct mtd_info *mtd, unsigned long len,
+ unsigned long offset, unsigned long flags);
+int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
+ u_char *buf);
+int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
+ const u_char *buf);
+int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
+ const u_char *buf);
static inline int mtd_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
@@ -405,27 +340,11 @@ static inline void mtd_sync(struct mtd_info *mtd)
mtd->_sync(mtd);
}
-/* Chip-supported device locking */
-static inline int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
- if (!mtd->_lock)
- return -EOPNOTSUPP;
- return mtd->_lock(mtd, ofs, len);
-}
-
-static inline int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
- if (!mtd->_unlock)
- return -EOPNOTSUPP;
- return mtd->_unlock(mtd, ofs, len);
-}
-
-static inline int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
-{
- if (!mtd->_is_locked)
- return -EOPNOTSUPP;
- return mtd->_is_locked(mtd, ofs, len);
-}
+int mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
+int mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
+int mtd_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len);
+int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs);
+int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs);
static inline int mtd_suspend(struct mtd_info *mtd)
{
@@ -438,20 +357,6 @@ static inline void mtd_resume(struct mtd_info *mtd)
mtd->_resume(mtd);
}
-static inline int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
-{
- if (!mtd->_block_isbad)
- return 0;
- return mtd->_block_isbad(mtd, ofs);
-}
-
-static inline int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
-{
- if (!mtd->_block_markbad)
- return -EOPNOTSUPP;
- return mtd->_block_markbad(mtd, ofs);
-}
-
static inline uint32_t mtd_div_by_eb(uint64_t sz, struct mtd_info *mtd)
{
if (mtd->erasesize_shift)
--
1.7.9
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] mtd: do not duplicate length and offset checks in drivers
2012-02-03 17:01 [PATCH 1/4] mtd: return error code from mtd_unpoint Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 2/4] mtd: add offset and length checks to the API function Artem Bityutskiy
@ 2012-02-03 17:01 ` Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 4/4] mtd: remove R/O checking duplication Artem Bityutskiy
2 siblings, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-02-03 17:01 UTC (permalink / raw)
To: MTD Maling List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
We already verify that offset and length are within the MTD device size
in the MTD API functions. Let's remove the duplicated checks in drivers.
This patch only affects the following API's:
'mtd_erase()'
'mtd_point()'
'mtd_unpoint()'
'mtd_get_unmapped_area()'
'mtd_read()'
'mtd_write()'
'mtd_panic_write()'
'mtd_lock()'
'mtd_unlock()'
'mtd_is_locked()'
'mtd_block_isbad()'
'mtd_block_markbad()'
This patch adds a bit of noise by removing too sparse empty lines, but this is
not too bad.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
drivers/mtd/chips/cfi_cmdset_0001.c | 2 +-
drivers/mtd/chips/cfi_cmdset_0020.c | 9 --------
drivers/mtd/chips/cfi_util.c | 6 -----
drivers/mtd/devices/block2mtd.c | 9 --------
drivers/mtd/devices/doc2000.c | 8 -------
drivers/mtd/devices/doc2001.c | 8 -------
drivers/mtd/devices/doc2001plus.c | 8 -------
drivers/mtd/devices/lart.c | 5 ----
drivers/mtd/devices/m25p80.c | 12 -----------
drivers/mtd/devices/ms02-nv.c | 8 -------
drivers/mtd/devices/mtd_dataflash.c | 7 ------
drivers/mtd/devices/mtdram.c | 17 ---------------
drivers/mtd/devices/phram.c | 23 ---------------------
drivers/mtd/devices/pmc551.c | 38 -----------------------------------
drivers/mtd/devices/slram.c | 23 ---------------------
drivers/mtd/devices/spear_smi.c | 12 -----------
drivers/mtd/devices/sst25l.c | 9 --------
drivers/mtd/lpddr/lpddr_cmds.c | 5 +---
drivers/mtd/mtdconcat.c | 26 -----------------------
drivers/mtd/mtdpart.c | 30 +--------------------------
drivers/mtd/nand/nand_base.c | 28 -------------------------
drivers/mtd/onenand/onenand_base.c | 20 ------------------
drivers/mtd/ubi/gluebi.c | 11 ----------
23 files changed, 3 insertions(+), 321 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 4d04551..27008ae 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1324,7 +1324,7 @@ static int cfi_intelext_point(struct mtd_info *mtd, loff_t from, size_t len,
int chipnum;
int ret = 0;
- if (!map->virt || (from + len > mtd->size))
+ if (!map->virt)
return -EINVAL;
/* Now lock the chip(s) to POINT state */
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 3861cca..160402f 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -904,12 +904,6 @@ static int cfi_staa_erase_varsize(struct mtd_info *mtd,
int i, first;
struct mtd_erase_region_info *regions = mtd->eraseregions;
- if (instr->addr > mtd->size)
- return -EINVAL;
-
- if ((instr->len + instr->addr) > mtd->size)
- return -EINVAL;
-
/* Check that both start and end of the requested erase are
* aligned with the erasesize at the appropriate addresses.
*/
@@ -1155,9 +1149,6 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
if (len & (mtd->erasesize -1))
return -EINVAL;
- if ((len + ofs) > mtd->size)
- return -EINVAL;
-
chipnum = ofs >> cfi->chipshift;
adr = ofs - (chipnum << cfi->chipshift);
diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c
index 8e46405..f992418 100644
--- a/drivers/mtd/chips/cfi_util.c
+++ b/drivers/mtd/chips/cfi_util.c
@@ -173,12 +173,6 @@ int cfi_varsize_frob(struct mtd_info *mtd, varsize_frob_t frob,
int i, first;
struct mtd_erase_region_info *regions = mtd->eraseregions;
- if (ofs > mtd->size)
- return -EINVAL;
-
- if ((len + ofs) > mtd->size)
- return -EINVAL;
-
/* Check that both start and end of the requested erase are
* aligned with the erasesize at the appropriate addresses.
*/
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 0fccf14..4c2f84c 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -104,11 +104,6 @@ static int block2mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
int offset = from & (PAGE_SIZE-1);
int cpylen;
- if (from > mtd->size)
- return -EINVAL;
- if (from + len > mtd->size)
- len = mtd->size - from;
-
if (retlen)
*retlen = 0;
@@ -190,10 +185,6 @@ static int block2mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
if (!len)
return 0;
- if (to >= mtd->size)
- return -ENOSPC;
- if (to + len > mtd->size)
- len = mtd->size - to;
mutex_lock(&dev->write_mutex);
err = _block2mtd_write(dev, buf, to, len, retlen);
diff --git a/drivers/mtd/devices/doc2000.c b/drivers/mtd/devices/doc2000.c
index 115d890..ee4ee0b 100644
--- a/drivers/mtd/devices/doc2000.c
+++ b/drivers/mtd/devices/doc2000.c
@@ -602,10 +602,6 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
int i, len256 = 0, ret=0;
size_t left = len;
- /* Don't allow read past end of device */
- if (from >= this->totlen)
- return -EINVAL;
-
mutex_lock(&this->lock);
*retlen = 0;
@@ -748,10 +744,6 @@ static int doc_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t left = len;
int status;
- /* Don't allow write past end of device */
- if (to >= this->totlen)
- return -EINVAL;
-
mutex_lock(&this->lock);
*retlen = 0;
diff --git a/drivers/mtd/devices/doc2001.c b/drivers/mtd/devices/doc2001.c
index b1185f9..1784415 100644
--- a/drivers/mtd/devices/doc2001.c
+++ b/drivers/mtd/devices/doc2001.c
@@ -383,10 +383,6 @@ static int doc_read (struct mtd_info *mtd, loff_t from, size_t len,
void __iomem *docptr = this->virtadr;
struct Nand *mychip = &this->chips[from >> (this->chipshift)];
- /* Don't allow read past end of device */
- if (from >= this->totlen)
- return -EINVAL;
-
/* Don't allow a single read to cross a 512-byte block boundary */
if (from + len > ((from | 0x1ff) + 1))
len = ((from | 0x1ff) + 1) - from;
@@ -494,10 +490,6 @@ static int doc_write (struct mtd_info *mtd, loff_t to, size_t len,
void __iomem *docptr = this->virtadr;
struct Nand *mychip = &this->chips[to >> (this->chipshift)];
- /* Don't allow write past end of device */
- if (to >= this->totlen)
- return -EINVAL;
-
#if 0
/* Don't allow a single write to cross a 512-byte block boundary */
if (to + len > ( (to | 0x1ff) + 1))
diff --git a/drivers/mtd/devices/doc2001plus.c b/drivers/mtd/devices/doc2001plus.c
index c9fbadd..a472bab 100644
--- a/drivers/mtd/devices/doc2001plus.c
+++ b/drivers/mtd/devices/doc2001plus.c
@@ -581,10 +581,6 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
void __iomem * docptr = this->virtadr;
struct Nand *mychip = &this->chips[from >> (this->chipshift)];
- /* Don't allow read past end of device */
- if (from >= this->totlen)
- return -EINVAL;
-
/* Don't allow a single read to cross a 512-byte block boundary */
if (from + len > ((from | 0x1ff) + 1))
len = ((from | 0x1ff) + 1) - from;
@@ -700,10 +696,6 @@ static int doc_write(struct mtd_info *mtd, loff_t to, size_t len,
void __iomem * docptr = this->virtadr;
struct Nand *mychip = &this->chips[to >> (this->chipshift)];
- /* Don't allow write past end of device */
- if (to >= this->totlen)
- return -EINVAL;
-
/* Don't allow writes which aren't exactly one block (512 bytes) */
if ((to & 0x1ff) || (len != 0x200))
return -EINVAL;
diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
index 6d6502c..c9ae601 100644
--- a/drivers/mtd/devices/lart.c
+++ b/drivers/mtd/devices/lart.c
@@ -367,9 +367,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
printk (KERN_DEBUG "%s(addr = 0x%.8x, len = %d)\n", __func__, instr->addr, instr->len);
#endif
- /* sanity checks */
- if (instr->addr + instr->len > mtd->size) return (-EINVAL);
-
/*
* check that both start and end of the requested erase are
* aligned with the erasesize at the appropriate addresses.
@@ -442,7 +439,6 @@ static int flash_read (struct mtd_info *mtd,loff_t from,size_t len,size_t *retle
/* sanity checks */
if (!len) return (0);
- if (from + len > mtd->size) return (-EINVAL);
/* we always read len bytes */
*retlen = len;
@@ -526,7 +522,6 @@ static int flash_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen
/* sanity checks */
if (!len) return (0);
- if (to + len > mtd->size) return (-EINVAL);
/* first, we write a 0xFF.... padded byte until we reach a dword boundary */
if (to & (BUSWIDTH - 1))
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 8808da9..0955a8f 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -288,9 +288,6 @@ static int m25p80_erase(struct mtd_info *mtd, struct erase_info *instr)
__func__, (long long)instr->addr,
(long long)instr->len);
- /* sanity checks */
- if (instr->addr + instr->len > flash->mtd.size)
- return -EINVAL;
div_u64_rem(instr->len, mtd->erasesize, &rem);
if (rem)
return -EINVAL;
@@ -353,9 +350,6 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
if (!len)
return 0;
- if (from + len > flash->mtd.size)
- return -EINVAL;
-
spi_message_init(&m);
memset(t, 0, (sizeof t));
@@ -423,9 +417,6 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
if (!len)
return(0);
- if (to + len > flash->mtd.size)
- return -EINVAL;
-
spi_message_init(&m);
memset(t, 0, (sizeof t));
@@ -515,9 +506,6 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
if (!len)
return 0;
- if (to + len > flash->mtd.size)
- return -EINVAL;
-
spi_message_init(&m);
memset(t, 0, (sizeof t));
diff --git a/drivers/mtd/devices/ms02-nv.c b/drivers/mtd/devices/ms02-nv.c
index 3a05af5..182849d 100644
--- a/drivers/mtd/devices/ms02-nv.c
+++ b/drivers/mtd/devices/ms02-nv.c
@@ -59,12 +59,8 @@ static int ms02nv_read(struct mtd_info *mtd, loff_t from,
{
struct ms02nv_private *mp = mtd->priv;
- if (from + len > mtd->size)
- return -EINVAL;
-
memcpy(buf, mp->uaddr + from, len);
*retlen = len;
-
return 0;
}
@@ -73,12 +69,8 @@ static int ms02nv_write(struct mtd_info *mtd, loff_t to,
{
struct ms02nv_private *mp = mtd->priv;
- if (to + len > mtd->size)
- return -EINVAL;
-
memcpy(mp->uaddr + to, buf, len);
*retlen = len;
-
return 0;
}
diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index fd4a9fc..fc5c781 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -164,9 +164,6 @@ static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
dev_name(&spi->dev), (long long)instr->addr,
(long long)instr->len);
- /* Sanity checks */
- if (instr->addr + instr->len > mtd->size)
- return -EINVAL;
div_u64_rem(instr->len, priv->page_size, &rem);
if (rem)
return -EINVAL;
@@ -257,8 +254,6 @@ static int dataflash_read(struct mtd_info *mtd, loff_t from, size_t len,
/* Sanity checks */
if (!len)
return 0;
- if (from + len > mtd->size)
- return -EINVAL;
/* Calculate flash page/byte address */
addr = (((unsigned)from / priv->page_size) << priv->page_offset)
@@ -333,8 +328,6 @@ static int dataflash_write(struct mtd_info *mtd, loff_t to, size_t len,
/* Sanity checks */
if (!len)
return 0;
- if ((to + len) > mtd->size)
- return -EINVAL;
spi_message_init(&msg);
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index e1f017b..0e0e6ed 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -34,27 +34,18 @@ static struct mtd_info *mtd_info;
static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
{
- if (instr->addr + instr->len > mtd->size)
- return -EINVAL;
-
memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
-
instr->state = MTD_ERASE_DONE;
mtd_erase_callback(instr);
-
return 0;
}
static int ram_point(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys)
{
- if (from + len > mtd->size)
- return -EINVAL;
-
/* can we return a physical address with this driver? */
if (phys)
return -EINVAL;
-
*virt = mtd->priv + from;
*retlen = len;
return 0;
@@ -81,11 +72,7 @@ static unsigned long ram_get_unmapped_area(struct mtd_info *mtd,
static int ram_read(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, u_char *buf)
{
- if (from + len > mtd->size)
- return -EINVAL;
-
memcpy(buf, mtd->priv + from, len);
-
*retlen = len;
return 0;
}
@@ -93,11 +80,7 @@ static int ram_read(struct mtd_info *mtd, loff_t from, size_t len,
static int ram_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
- if (to + len > mtd->size)
- return -EINVAL;
-
memcpy((char *)mtd->priv + to, buf, len);
-
*retlen = len;
return 0;
}
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 3803555..36add7c 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -38,29 +38,20 @@ static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
{
u_char *start = mtd->priv;
- if (instr->addr + instr->len > mtd->size)
- return -EINVAL;
-
memset(start + instr->addr, 0xff, instr->len);
/* This'll catch a few races. Free the thing before returning :)
* I don't feel at all ashamed. This kind of thing is possible anyway
* with flash, but unlikely.
*/
-
instr->state = MTD_ERASE_DONE;
-
mtd_erase_callback(instr);
-
return 0;
}
static int phram_point(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys)
{
- if (from + len > mtd->size)
- return -EINVAL;
-
/* can we return a physical address with this driver? */
if (phys)
return -EINVAL;
@@ -80,14 +71,7 @@ static int phram_read(struct mtd_info *mtd, loff_t from, size_t len,
{
u_char *start = mtd->priv;
- if (from >= mtd->size)
- return -EINVAL;
-
- if (len > mtd->size - from)
- len = mtd->size - from;
-
memcpy(buf, start + from, len);
-
*retlen = len;
return 0;
}
@@ -97,14 +81,7 @@ static int phram_write(struct mtd_info *mtd, loff_t to, size_t len,
{
u_char *start = mtd->priv;
- if (to >= mtd->size)
- return -EINVAL;
-
- if (len > mtd->size - to)
- len = mtd->size - to;
-
memcpy(start + to, buf, len);
-
*retlen = len;
return 0;
}
diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
index 40a96c7..725198d 100644
--- a/drivers/mtd/devices/pmc551.c
+++ b/drivers/mtd/devices/pmc551.c
@@ -116,16 +116,6 @@ static int pmc551_erase(struct mtd_info *mtd, struct erase_info *instr)
#endif
end = instr->addr + instr->len - 1;
-
- /* Is it past the end? */
- if (end > mtd->size) {
-#ifdef CONFIG_MTD_PMC551_DEBUG
- printk(KERN_DEBUG "pmc551_erase() out of bounds (%ld > %ld)\n",
- (long)end, (long)mtd->size);
-#endif
- return -EINVAL;
- }
-
eoff_hi = end & ~(priv->asize - 1);
soff_hi = instr->addr & ~(priv->asize - 1);
eoff_lo = end & (priv->asize - 1);
@@ -179,14 +169,6 @@ static int pmc551_point(struct mtd_info *mtd, loff_t from, size_t len,
printk(KERN_DEBUG "pmc551_point(%ld, %ld)\n", (long)from, (long)len);
#endif
- if (from + len > mtd->size) {
-#ifdef CONFIG_MTD_PMC551_DEBUG
- printk(KERN_DEBUG "pmc551_point() out of bounds (%ld > %ld)\n",
- (long)from + len, (long)mtd->size);
-#endif
- return -EINVAL;
- }
-
/* can we return a physical address with this driver? */
if (phys)
return -EINVAL;
@@ -229,16 +211,6 @@ static int pmc551_read(struct mtd_info *mtd, loff_t from, size_t len,
#endif
end = from + len - 1;
-
- /* Is it past the end? */
- if (end > mtd->size) {
-#ifdef CONFIG_MTD_PMC551_DEBUG
- printk(KERN_DEBUG "pmc551_read() out of bounds (%ld > %ld)\n",
- (long)end, (long)mtd->size);
-#endif
- return -EINVAL;
- }
-
soff_hi = from & ~(priv->asize - 1);
eoff_hi = end & ~(priv->asize - 1);
soff_lo = from & (priv->asize - 1);
@@ -296,16 +268,6 @@ static int pmc551_write(struct mtd_info *mtd, loff_t to, size_t len,
#endif
end = to + len - 1;
- /* Is it past the end? or did the u32 wrap? */
- if (end > mtd->size) {
-#ifdef CONFIG_MTD_PMC551_DEBUG
- printk(KERN_DEBUG "pmc551_write() out of bounds (end: %ld, "
- "size: %ld, to: %ld)\n", (long)end, (long)mtd->size,
- (long)to);
-#endif
- return -EINVAL;
- }
-
soff_hi = to & ~(priv->asize - 1);
eoff_hi = end & ~(priv->asize - 1);
soff_lo = to & (priv->asize - 1);
diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
index 9431ffc..842e489 100644
--- a/drivers/mtd/devices/slram.c
+++ b/drivers/mtd/devices/slram.c
@@ -84,21 +84,13 @@ static int slram_erase(struct mtd_info *mtd, struct erase_info *instr)
{
slram_priv_t *priv = mtd->priv;
- if (instr->addr + instr->len > mtd->size) {
- return(-EINVAL);
- }
-
memset(priv->start + instr->addr, 0xff, instr->len);
-
/* This'll catch a few races. Free the thing before returning :)
* I don't feel at all ashamed. This kind of thing is possible anyway
* with flash, but unlikely.
*/
-
instr->state = MTD_ERASE_DONE;
-
mtd_erase_callback(instr);
-
return(0);
}
@@ -110,10 +102,6 @@ static int slram_point(struct mtd_info *mtd, loff_t from, size_t len,
/* can we return a physical address with this driver? */
if (phys)
return -EINVAL;
-
- if (from + len > mtd->size)
- return -EINVAL;
-
*virt = priv->start + from;
*retlen = len;
return(0);
@@ -129,14 +117,7 @@ static int slram_read(struct mtd_info *mtd, loff_t from, size_t len,
{
slram_priv_t *priv = mtd->priv;
- if (from > mtd->size)
- return -EINVAL;
-
- if (from + len > mtd->size)
- len = mtd->size - from;
-
memcpy(buf, priv->start + from, len);
-
*retlen = len;
return(0);
}
@@ -146,11 +127,7 @@ static int slram_write(struct mtd_info *mtd, loff_t to, size_t len,
{
slram_priv_t *priv = mtd->priv;
- if (to + len > mtd->size)
- return -EINVAL;
-
memcpy(priv->start + to, buf, len);
-
*retlen = len;
return(0);
}
diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c
index b4c02d6..f7b9949 100644
--- a/drivers/mtd/devices/spear_smi.c
+++ b/drivers/mtd/devices/spear_smi.c
@@ -512,10 +512,6 @@ static int spear_mtd_erase(struct mtd_info *mtd, struct erase_info *e_info)
if (!flash || !dev)
return -ENODEV;
- /* do not allow erase past end of device */
- if (e_info->addr + e_info->len > flash->mtd.size)
- return -EINVAL;
-
bank = flash->bank;
if (bank > dev->num_flashes - 1) {
dev_err(&dev->pdev->dev, "Invalid Bank Num");
@@ -575,10 +571,6 @@ static int spear_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
if (!flash || !dev)
return -ENODEV;
- /* do not allow reads past end of device */
- if (from + len > flash->mtd.size)
- return -EINVAL;
-
if (flash->bank > dev->num_flashes - 1) {
dev_err(&dev->pdev->dev, "Invalid Bank Num");
return -EINVAL;
@@ -680,10 +672,6 @@ static int spear_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
if (!len)
return 0;
- /* do not allow write past end of page */
- if (to + len > flash->mtd.size)
- return -EINVAL;
-
if (flash->bank > dev->num_flashes - 1) {
dev_err(&dev->pdev->dev, "Invalid Bank Num");
return -EINVAL;
diff --git a/drivers/mtd/devices/sst25l.c b/drivers/mtd/devices/sst25l.c
index 8b9ffaf..99d4a3c 100644
--- a/drivers/mtd/devices/sst25l.c
+++ b/drivers/mtd/devices/sst25l.c
@@ -175,9 +175,6 @@ static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr)
int err;
/* Sanity checks */
- if (instr->addr + instr->len > flash->mtd.size)
- return -EINVAL;
-
if ((uint32_t)instr->len % mtd->erasesize)
return -EINVAL;
@@ -227,9 +224,6 @@ static int sst25l_read(struct mtd_info *mtd, loff_t from, size_t len,
if (len == 0)
return 0;
- if (from + len > flash->mtd.size)
- return -EINVAL;
-
if (retlen)
*retlen = 0;
@@ -278,9 +272,6 @@ static int sst25l_write(struct mtd_info *mtd, loff_t to, size_t len,
if (!len)
return 0;
- if (to + len > flash->mtd.size)
- return -EINVAL;
-
if ((uint32_t)to % mtd->writesize)
return -EINVAL;
diff --git a/drivers/mtd/lpddr/lpddr_cmds.c b/drivers/mtd/lpddr/lpddr_cmds.c
index de960b1..0f3731c 100644
--- a/drivers/mtd/lpddr/lpddr_cmds.c
+++ b/drivers/mtd/lpddr/lpddr_cmds.c
@@ -530,7 +530,7 @@ static int lpddr_point(struct mtd_info *mtd, loff_t adr, size_t len,
struct flchip *chip = &lpddr->chips[chipnum];
int ret = 0;
- if (!map->virt || (adr + len > mtd->size))
+ if (!map->virt)
return -EINVAL;
/* ofs: offset within the first chip that the first read should start */
@@ -692,9 +692,6 @@ static int lpddr_erase(struct mtd_info *mtd, struct erase_info *instr)
ofs = instr->addr;
len = instr->len;
- if (ofs > mtd->size || (len + ofs) > mtd->size)
- return -EINVAL;
-
while (len > 0) {
ret = do_erase_oneblock(mtd, ofs);
if (ret)
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index d826a8a..1f20718 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -185,10 +185,6 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
for (i = 0; i < count; i++)
total_len += vecs[i].iov_len;
- /* Do not allow write past end of device */
- if ((to + total_len) > mtd->size)
- return -EINVAL;
-
/* Check alignment */
if (mtd->writesize > 1) {
uint64_t __to = to;
@@ -406,12 +402,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
- if (instr->addr > concat->mtd.size)
- return -EINVAL;
-
- if (instr->len + instr->addr > concat->mtd.size)
- return -EINVAL;
-
/*
* Check for proper erase block alignment of the to-be-erased area.
* It is easier to do this based on the super device's erase
@@ -538,9 +528,6 @@ static int concat_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
struct mtd_concat *concat = CONCAT(mtd);
int i, err = -EINVAL;
- if ((len + ofs) > mtd->size)
- return -EINVAL;
-
for (i = 0; i < concat->num_subdev; i++) {
struct mtd_info *subdev = concat->subdev[i];
uint64_t size;
@@ -575,9 +562,6 @@ static int concat_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
struct mtd_concat *concat = CONCAT(mtd);
int i, err = 0;
- if ((len + ofs) > mtd->size)
- return -EINVAL;
-
for (i = 0; i < concat->num_subdev; i++) {
struct mtd_info *subdev = concat->subdev[i];
uint64_t size;
@@ -650,9 +634,6 @@ static int concat_block_isbad(struct mtd_info *mtd, loff_t ofs)
if (!mtd_can_have_bb(concat->subdev[0]))
return res;
- if (ofs > mtd->size)
- return -EINVAL;
-
for (i = 0; i < concat->num_subdev; i++) {
struct mtd_info *subdev = concat->subdev[i];
@@ -673,9 +654,6 @@ static int concat_block_markbad(struct mtd_info *mtd, loff_t ofs)
struct mtd_concat *concat = CONCAT(mtd);
int i, err = -EINVAL;
- if (ofs > mtd->size)
- return -EINVAL;
-
for (i = 0; i < concat->num_subdev; i++) {
struct mtd_info *subdev = concat->subdev[i];
@@ -713,10 +691,6 @@ static unsigned long concat_get_unmapped_area(struct mtd_info *mtd,
continue;
}
- /* we've found the subdev over which the mapping will reside */
- if (offset + len > subdev->size)
- return (unsigned long) -EINVAL;
-
return mtd_get_unmapped_area(subdev, len, offset, flags);
}
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index da8a0b2..fbe2c8a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -65,11 +65,6 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
int res;
stats = part->master->ecc_stats;
-
- if (from >= mtd->size)
- len = 0;
- else if (from + len > mtd->size)
- len = mtd->size - from;
res = mtd_read(part->master, from + part->offset, len, retlen, buf);
if (unlikely(res)) {
if (mtd_is_bitflip(res))
@@ -84,10 +79,7 @@ static int part_point(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys)
{
struct mtd_part *part = PART(mtd);
- if (from >= mtd->size)
- len = 0;
- else if (from + len > mtd->size)
- len = mtd->size - from;
+
return mtd_point(part->master, from + part->offset, len, retlen,
virt, phys);
}
@@ -182,10 +174,6 @@ static int part_write(struct mtd_info *mtd, loff_t to, size_t len,
struct mtd_part *part = PART(mtd);
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
- if (to >= mtd->size)
- len = 0;
- else if (to + len > mtd->size)
- len = mtd->size - to;
return mtd_write(part->master, to + part->offset, len, retlen, buf);
}
@@ -195,10 +183,6 @@ static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
struct mtd_part *part = PART(mtd);
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
- if (to >= mtd->size)
- len = 0;
- else if (to + len > mtd->size)
- len = mtd->size - to;
return mtd_panic_write(part->master, to + part->offset, len, retlen,
buf);
}
@@ -248,8 +232,6 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
int ret;
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
- if (instr->addr >= mtd->size)
- return -EINVAL;
instr->addr += part->offset;
ret = mtd_erase(part->master, instr);
if (ret) {
@@ -277,24 +259,18 @@ EXPORT_SYMBOL_GPL(mtd_erase_callback);
static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct mtd_part *part = PART(mtd);
- if ((len + ofs) > mtd->size)
- return -EINVAL;
return mtd_lock(part->master, ofs + part->offset, len);
}
static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct mtd_part *part = PART(mtd);
- if ((len + ofs) > mtd->size)
- return -EINVAL;
return mtd_unlock(part->master, ofs + part->offset, len);
}
static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct mtd_part *part = PART(mtd);
- if ((len + ofs) > mtd->size)
- return -EINVAL;
return mtd_is_locked(part->master, ofs + part->offset, len);
}
@@ -319,8 +295,6 @@ static void part_resume(struct mtd_info *mtd)
static int part_block_isbad(struct mtd_info *mtd, loff_t ofs)
{
struct mtd_part *part = PART(mtd);
- if (ofs >= mtd->size)
- return -EINVAL;
ofs += part->offset;
return mtd_block_isbad(part->master, ofs);
}
@@ -332,8 +306,6 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
- if (ofs >= mtd->size)
- return -EINVAL;
ofs += part->offset;
res = mtd_block_markbad(part->master, ofs);
if (!res)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 05f8243..daaa6a5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -123,12 +123,6 @@ static int check_offs_len(struct mtd_info *mtd,
ret = -EINVAL;
}
- /* Do not allow past end of device */
- if (ofs + len > mtd->size) {
- pr_debug("%s: past end of device\n", __func__);
- ret = -EINVAL;
- }
-
return ret;
}
@@ -1608,25 +1602,17 @@ static int nand_read(struct mtd_info *mtd, loff_t from, size_t len,
struct mtd_oob_ops ops;
int ret;
- /* Do not allow reads past end of device */
- if ((from + len) > mtd->size)
- return -EINVAL;
if (!len)
return 0;
nand_get_device(chip, mtd, FL_READING);
-
ops.len = len;
ops.datbuf = buf;
ops.oobbuf = NULL;
ops.mode = 0;
-
ret = nand_do_read_ops(mtd, from, &ops);
-
*retlen = ops.retlen;
-
nand_release_device(mtd);
-
return ret;
}
@@ -2316,8 +2302,6 @@ static int panic_nand_write(struct mtd_info *mtd, loff_t to, size_t len,
int ret;
/* Do not allow reads past end of device */
- if ((to + len) > mtd->size)
- return -EINVAL;
if (!len)
return 0;
@@ -2355,25 +2339,17 @@ static int nand_write(struct mtd_info *mtd, loff_t to, size_t len,
struct mtd_oob_ops ops;
int ret;
- /* Do not allow reads past end of device */
- if ((to + len) > mtd->size)
- return -EINVAL;
if (!len)
return 0;
nand_get_device(chip, mtd, FL_WRITING);
-
ops.len = len;
ops.datbuf = (uint8_t *)buf;
ops.oobbuf = NULL;
ops.mode = 0;
-
ret = nand_do_write_ops(mtd, to, &ops);
-
*retlen = ops.retlen;
-
nand_release_device(mtd);
-
return ret;
}
@@ -2737,10 +2713,6 @@ static void nand_sync(struct mtd_info *mtd)
*/
static int nand_block_isbad(struct mtd_info *mtd, loff_t offs)
{
- /* Check for invalid offset */
- if (offs > mtd->size)
- return -EINVAL;
-
return nand_block_checkbad(mtd, offs, 1, 0);
}
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 914c49b..9c6445d 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1756,13 +1756,6 @@ static int onenand_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
/* Initialize retlen, in case of early exit */
*retlen = 0;
- /* Do not allow writes past end of device */
- if (unlikely((to + len) > mtd->size)) {
- printk(KERN_ERR "%s: Attempt write to past end of device\n",
- __func__);
- return -EINVAL;
- }
-
/* Reject writes, which are not page aligned */
if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
printk(KERN_ERR "%s: Attempt to write not page aligned data\n",
@@ -1890,13 +1883,6 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to,
ops->retlen = 0;
ops->oobretlen = 0;
- /* Do not allow writes past end of device */
- if (unlikely((to + len) > mtd->size)) {
- printk(KERN_ERR "%s: Attempt write to past end of device\n",
- __func__);
- return -EINVAL;
- }
-
/* Reject writes, which are not page aligned */
if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
printk(KERN_ERR "%s: Attempt to write not page aligned data\n",
@@ -2493,12 +2479,6 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
(unsigned long long)instr->addr,
(unsigned long long)instr->len);
- /* Do not allow erase past end of device */
- if (unlikely((len + addr) > mtd->size)) {
- printk(KERN_ERR "%s: Erase past end of device\n", __func__);
- return -EINVAL;
- }
-
if (FLEXONENAND(this)) {
/* Find the eraseregion of this address */
int i = flexonenand_region(mtd, addr);
diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
index 0101dce..b875c2c 100644
--- a/drivers/mtd/ubi/gluebi.c
+++ b/drivers/mtd/ubi/gluebi.c
@@ -174,11 +174,7 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
int err = 0, lnum, offs, total_read;
struct gluebi_device *gluebi;
- if (len < 0 || from < 0 || from + len > mtd->size)
- return -EINVAL;
-
gluebi = container_of(mtd, struct gluebi_device, mtd);
-
lnum = div_u64_rem(from, mtd->erasesize, &offs);
total_read = len;
while (total_read) {
@@ -218,9 +214,6 @@ static int gluebi_write(struct mtd_info *mtd, loff_t to, size_t len,
int err = 0, lnum, offs, total_written;
struct gluebi_device *gluebi;
- if (len < 0 || to < 0 || len + to > mtd->size)
- return -EINVAL;
-
gluebi = container_of(mtd, struct gluebi_device, mtd);
if (!(mtd->flags & MTD_WRITEABLE))
@@ -265,10 +258,6 @@ static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr)
int err, i, lnum, count;
struct gluebi_device *gluebi;
- if (instr->addr < 0 || instr->addr > mtd->size - mtd->erasesize)
- return -EINVAL;
- if (instr->len < 0 || instr->addr + instr->len > mtd->size)
- return -EINVAL;
if (mtd_mod_by_ws(instr->addr, mtd) || mtd_mod_by_ws(instr->len, mtd))
return -EINVAL;
--
1.7.9
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] mtd: remove R/O checking duplication
2012-02-03 17:01 [PATCH 1/4] mtd: return error code from mtd_unpoint Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 2/4] mtd: add offset and length checks to the API function Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 3/4] mtd: do not duplicate length and offset checks in drivers Artem Bityutskiy
@ 2012-02-03 17:01 ` Artem Bityutskiy
2012-02-05 8:33 ` Shmulik Ladkani
2 siblings, 1 reply; 6+ messages in thread
From: Artem Bityutskiy @ 2012-02-03 17:01 UTC (permalink / raw)
To: MTD Maling List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Many drivers check whether the partition is R/O and return -EROFS if yes.
Let's stop having duplicated checks and move them to the API functions
instead.
And again a bit of noise - deleted few too sparse newlines, sorry.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
drivers/mtd/chips/map_ram.c | 4 ----
drivers/mtd/chips/map_rom.c | 3 +--
drivers/mtd/devices/phram.c | 1 -
drivers/mtd/mtdconcat.c | 27 +++------------------------
drivers/mtd/mtdcore.c | 10 +++++++++-
drivers/mtd/mtdpart.c | 14 +-------------
drivers/mtd/ubi/gluebi.c | 8 --------
include/linux/mtd/mtd.h | 2 ++
8 files changed, 16 insertions(+), 53 deletions(-)
diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
index 2253070..991c2a1 100644
--- a/drivers/mtd/chips/map_ram.c
+++ b/drivers/mtd/chips/map_ram.c
@@ -122,14 +122,10 @@ static int mapram_erase (struct mtd_info *mtd, struct erase_info *instr)
unsigned long i;
allff = map_word_ff(map);
-
for (i=0; i<instr->len; i += map_bankwidth(map))
map_write(map, allff, instr->addr + i);
-
instr->state = MTD_ERASE_DONE;
-
mtd_erase_callback(instr);
-
return 0;
}
diff --git a/drivers/mtd/chips/map_rom.c b/drivers/mtd/chips/map_rom.c
index facb560..47a43cf 100644
--- a/drivers/mtd/chips/map_rom.c
+++ b/drivers/mtd/chips/map_rom.c
@@ -85,8 +85,7 @@ static void maprom_nop(struct mtd_info *mtd)
static int maprom_write (struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf)
{
- printk(KERN_NOTICE "maprom_write called\n");
- return -EIO;
+ return -EROFS;
}
static int maprom_erase (struct mtd_info *mtd, struct erase_info *info)
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 36add7c..d0e8dc6 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -33,7 +33,6 @@ struct phram_mtd_list {
static LIST_HEAD(phram_list);
-
static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
{
u_char *start = mtd->priv;
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index 1f20718..dd24232 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -126,9 +126,6 @@ concat_write(struct mtd_info *mtd, loff_t to, size_t len,
int err = -EINVAL;
int i;
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
*retlen = 0;
for (i = 0; i < concat->num_subdev; i++) {
@@ -145,11 +142,7 @@ concat_write(struct mtd_info *mtd, loff_t to, size_t len,
else
size = len;
- if (!(subdev->flags & MTD_WRITEABLE))
- err = -EROFS;
- else
- err = mtd_write(subdev, to, size, &retsize, buf);
-
+ err = mtd_write(subdev, to, size, &retsize, buf);
if (err)
break;
@@ -176,9 +169,6 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
int i;
int err = -EINVAL;
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
*retlen = 0;
/* Calculate total length of data */
@@ -220,12 +210,8 @@ concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
old_iov_len = vecs_copy[entry_high].iov_len;
vecs_copy[entry_high].iov_len = size;
- if (!(subdev->flags & MTD_WRITEABLE))
- err = -EROFS;
- else
- err = mtd_writev(subdev, &vecs_copy[entry_low],
- entry_high - entry_low + 1, to,
- &retsize);
+ err = mtd_writev(subdev, &vecs_copy[entry_low],
+ entry_high - entry_low + 1, to, &retsize);
vecs_copy[entry_high].iov_len = old_iov_len - size;
vecs_copy[entry_high].iov_base += size;
@@ -399,9 +385,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
uint64_t length, offset = 0;
struct erase_info *erase;
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
/*
* Check for proper erase block alignment of the to-be-erased area.
* It is easier to do this based on the super device's erase
@@ -489,10 +472,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
else
erase->len = length;
- if (!(subdev->flags & MTD_WRITEABLE)) {
- err = -EROFS;
- break;
- }
length -= erase->len;
if ((err = concat_dev_erase(subdev, erase))) {
/* sanity check: should never happen since
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 13bd4fa..78e1df6 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -693,6 +693,8 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
{
if (instr->addr > mtd->size || instr->len > mtd->size - instr->addr)
return -EINVAL;
+ if (!(mtd->flags & MTD_WRITEABLE))
+ return -EROFS;
return mtd->_erase(mtd, instr);
}
EXPORT_SYMBOL_GPL(mtd_erase);
@@ -754,7 +756,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
const u_char *buf)
{
*retlen = 0;
- if (!mtd->_write)
+ if (!mtd->_write || !(mtd->flags & MTD_WRITEABLE))
return -EROFS;
if (to < 0 || to > mtd->size || len > mtd->size - to)
return -EINVAL;
@@ -777,6 +779,8 @@ int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
return -EOPNOTSUPP;
if (to < 0 || to > mtd->size || len > mtd->size - to)
return -EINVAL;
+ if (!(mtd->flags & MTD_WRITEABLE))
+ return -EROFS;
return mtd->_panic_write(mtd, to, len, retlen, buf);
}
EXPORT_SYMBOL_GPL(mtd_panic_write);
@@ -828,6 +832,8 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
return -EOPNOTSUPP;
if (ofs < 0 || ofs > mtd->size)
return -EINVAL;
+ if (!(mtd->flags & MTD_WRITEABLE))
+ return -EROFS;
return mtd->_block_markbad(mtd, ofs);
}
EXPORT_SYMBOL_GPL(mtd_block_markbad);
@@ -879,6 +885,8 @@ int mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
unsigned long count, loff_t to, size_t *retlen)
{
*retlen = 0;
+ if (!(mtd->flags & MTD_WRITEABLE))
+ return -EROFS;
if (!mtd->_writev)
return default_mtd_writev(mtd, vecs, count, to, retlen);
return mtd->_writev(mtd, vecs, count, to, retlen);
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index fbe2c8a..33d32c6 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -172,8 +172,6 @@ static int part_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
struct mtd_part *part = PART(mtd);
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
return mtd_write(part->master, to + part->offset, len, retlen, buf);
}
@@ -181,8 +179,6 @@ static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
struct mtd_part *part = PART(mtd);
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
return mtd_panic_write(part->master, to + part->offset, len, retlen,
buf);
}
@@ -192,9 +188,6 @@ static int part_write_oob(struct mtd_info *mtd, loff_t to,
{
struct mtd_part *part = PART(mtd);
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
if (to >= mtd->size)
return -EINVAL;
if (ops->datbuf && to + ops->len > mtd->size)
@@ -220,8 +213,6 @@ static int part_writev(struct mtd_info *mtd, const struct kvec *vecs,
unsigned long count, loff_t to, size_t *retlen)
{
struct mtd_part *part = PART(mtd);
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
return mtd_writev(part->master, vecs, count, to + part->offset,
retlen);
}
@@ -230,8 +221,7 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
{
struct mtd_part *part = PART(mtd);
int ret;
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
+
instr->addr += part->offset;
ret = mtd_erase(part->master, instr);
if (ret) {
@@ -304,8 +294,6 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
struct mtd_part *part = PART(mtd);
int res;
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
ofs += part->offset;
res = mtd_block_markbad(part->master, ofs);
if (!res)
diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
index b875c2c..90b9882 100644
--- a/drivers/mtd/ubi/gluebi.c
+++ b/drivers/mtd/ubi/gluebi.c
@@ -215,10 +215,6 @@ static int gluebi_write(struct mtd_info *mtd, loff_t to, size_t len,
struct gluebi_device *gluebi;
gluebi = container_of(mtd, struct gluebi_device, mtd);
-
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
lnum = div_u64_rem(to, mtd->erasesize, &offs);
if (len % mtd->writesize || offs % mtd->writesize)
@@ -263,12 +259,8 @@ static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr)
lnum = mtd_div_by_eb(instr->addr, mtd);
count = mtd_div_by_eb(instr->len, mtd);
-
gluebi = container_of(mtd, struct gluebi_device, mtd);
- if (!(mtd->flags & MTD_WRITEABLE))
- return -EROFS;
-
for (i = 0; i < count - 1; i++) {
err = ubi_leb_unmap(gluebi->desc, lnum + i);
if (err)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 317a80c..fa20a8f 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -268,6 +268,8 @@ static inline int mtd_write_oob(struct mtd_info *mtd, loff_t to,
ops->retlen = ops->oobretlen = 0;
if (!mtd->_write_oob)
return -EOPNOTSUPP;
+ if (!(mtd->flags & MTD_WRITEABLE))
+ return -EROFS;
return mtd->_write_oob(mtd, to, ops);
}
--
1.7.9
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] mtd: remove R/O checking duplication
2012-02-03 17:01 ` [PATCH 4/4] mtd: remove R/O checking duplication Artem Bityutskiy
@ 2012-02-05 8:33 ` Shmulik Ladkani
2012-02-06 9:12 ` Artem Bityutskiy
0 siblings, 1 reply; 6+ messages in thread
From: Shmulik Ladkani @ 2012-02-05 8:33 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: MTD Maling List
On Fri, 3 Feb 2012 19:01:31 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 13bd4fa..78e1df6 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -754,7 +756,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
> const u_char *buf)
> {
> *retlen = 0;
> - if (!mtd->_write)
> + if (!mtd->_write || !(mtd->flags & MTD_WRITEABLE))
> return -EROFS;
> if (to < 0 || to > mtd->size || len > mtd->size - to)
> return -EINVAL;
(sorry for nit-picking)
Here you perform "writable" (EROFS) validation prior offset/length
validation (EINVAL) - however in other API funtions you had it
vice-versa.
Really not that important, but better if handling is consistent.
Regards,
Shmulik
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] mtd: remove R/O checking duplication
2012-02-05 8:33 ` Shmulik Ladkani
@ 2012-02-06 9:12 ` Artem Bityutskiy
0 siblings, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-02-06 9:12 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: MTD Maling List
[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]
On Sun, 2012-02-05 at 10:33 +0200, Shmulik Ladkani wrote:
> On Fri, 3 Feb 2012 19:01:31 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> > index 13bd4fa..78e1df6 100644
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -754,7 +756,7 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen,
> > const u_char *buf)
> > {
> > *retlen = 0;
> > - if (!mtd->_write)
> > + if (!mtd->_write || !(mtd->flags & MTD_WRITEABLE))
> > return -EROFS;
> > if (to < 0 || to > mtd->size || len > mtd->size - to)
> > return -EINVAL;
>
> (sorry for nit-picking)
No need to sorry, you are very welcome - I am happy when people spot
issues before patches get merged.
> Here you perform "writable" (EROFS) validation prior offset/length
> validation (EINVAL) - however in other API funtions you had it
> vice-versa.
> Really not that important, but better if handling is consistent.
I think this is rather important to first validate the input parameters
and then check for R/O. I've done this in other functions, but failed to
do in 'mtd_write()'. Fixed in l2-mtd.git, thanks!
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-06 9:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-03 17:01 [PATCH 1/4] mtd: return error code from mtd_unpoint Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 2/4] mtd: add offset and length checks to the API function Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 3/4] mtd: do not duplicate length and offset checks in drivers Artem Bityutskiy
2012-02-03 17:01 ` [PATCH 4/4] mtd: remove R/O checking duplication Artem Bityutskiy
2012-02-05 8:33 ` Shmulik Ladkani
2012-02-06 9:12 ` Artem Bityutskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox