[PATCH v2] mtd: Verify written data in paranoid mode

[PATCH v2] mtd: Verify written data in paranoid mode

[Bence Csókás]

From: Csókás, Bence <csokas.bence@prolan.hu>

Add MTD_PARANOID config option for verifying all written data to prevent
silent bit errors being undetected, at the cost of some bandwidth overhead.

Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
---

Notes:
    Changes in v2:
    * refactor to be in mtdcore instead of spi-nor core

 drivers/mtd/Kconfig   | 14 ++++++++++++
 drivers/mtd/mtdcore.c | 51 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 796a2eccbef0..e75f4a57df6a 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -206,6 +206,20 @@ config MTD_PARTITIONED_MASTER
 	  the parent of the partition device be the master device, rather than
 	  what lies behind the master.
 
+config MTD_PARANOID
+	bool "Read back written data (paranoid mode)"
+	help
+	  This option makes the MTD core read back all data on a write and
+	  report an error if it doesn't match the written data. This can
+	  safeguard against silent bit errors resulting from a faulty Flash,
+	  controller oddities, bus noise etc.
+
+	  It is up to the layer above MTD (e.g. the filesystem) to handle
+	  this condition, for example by going read-only to prevent further
+	  data corruption, or to mark a certain region of Flash as bad.
+
+	  If you are unsure, select 'n'.
+
 source "drivers/mtd/chips/Kconfig"
 
 source "drivers/mtd/maps/Kconfig"
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5ba9a741f5ac..139cbac51132 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1745,8 +1745,8 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 }
 EXPORT_SYMBOL_GPL(mtd_read_oob);
 
-int mtd_write_oob(struct mtd_info *mtd, loff_t to,
-				struct mtd_oob_ops *ops)
+static int _mtd_write_oob(struct mtd_info *mtd, loff_t to,
+			  struct mtd_oob_ops *ops)
 {
 	struct mtd_info *master = mtd_get_master(mtd);
 	int ret;
@@ -1771,6 +1771,53 @@ int mtd_write_oob(struct mtd_info *mtd, loff_t to,
 
 	return mtd_write_oob_std(mtd, to, ops);
 }
+
+static int _mtd_verify(struct mtd_info *mtd, loff_t to, size_t len, const u8 *buf)
+{
+	struct device *dev = &mtd->dev;
+	u_char *verify_buf;
+	size_t r_retlen;
+	int ret;
+
+	verify_buf = devm_kmalloc(dev, len, GFP_KERNEL);
+	if (!verify_buf)
+		return -ENOMEM;
+
+	ret = mtd_read(mtd, to, len, &r_retlen, verify_buf);
+	if (ret < 0)
+		goto err;
+
+	if (len != r_retlen) {
+		/* We shouldn't see short reads */
+		dev_err(dev, "Verify failed, written %zd but only read %zd",
+			len, r_retlen);
+		ret = -EIO;
+		goto err;
+	}
+
+	if (memcmp(verify_buf, buf, len)) {
+		dev_err(dev, "Verify failed, compare mismatch!");
+		ret = -EIO;
+	}
+
+err:
+	devm_kfree(dev, verify_buf);
+	return ret;
+}
+
+int mtd_write_oob(struct mtd_info *mtd, loff_t to,
+		  struct mtd_oob_ops *ops)
+{
+	int ret = _mtd_write_oob(mtd, to, ops);
+
+#if IS_ENABLED(CONFIG_MTD_PARANOID)
+	if (ret < 0)
+		return ret;
+
+	ret = _mtd_verify(mtd, to, ops->retlen, ops->datbuf);
+#endif // CONFIG_MTD_PARANOID
+	return ret;
+}
 EXPORT_SYMBOL_GPL(mtd_write_oob);
 
 /**

base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
-- 
2.49.0



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: Verify written data in paranoid mode

[kernel test robot]

Hi Bence,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3]

url:    https://github.com/intel-lab-lkp/linux/commits/Bence-Cs-k-s/mtd-Verify-written-data-in-paranoid-mode/20250509-024044
base:   d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
patch link:    https://lore.kernel.org/r/20250508183733.514124-3-csokas.bence%40prolan.hu
patch subject: [PATCH v2] mtd: Verify written data in paranoid mode
config: i386-buildonly-randconfig-002-20250509 (https://download.01.org/0day-ci/archive/20250509/202505091318.0QvCicb9-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250509/202505091318.0QvCicb9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505091318.0QvCicb9-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mtd/mtdcore.c:1775:12: warning: '_mtd_verify' defined but not used [-Wunused-function]
    1775 | static int _mtd_verify(struct mtd_info *mtd, loff_t to, size_t len, const u8 *buf)
         |            ^~~~~~~~~~~


vim +/_mtd_verify +1775 drivers/mtd/mtdcore.c

  1774	
> 1775	static int _mtd_verify(struct mtd_info *mtd, loff_t to, size_t len, const u8 *buf)
  1776	{
  1777		struct device *dev = &mtd->dev;
  1778		u_char *verify_buf;
  1779		size_t r_retlen;
  1780		int ret;
  1781	
  1782		verify_buf = devm_kmalloc(dev, len, GFP_KERNEL);
  1783		if (!verify_buf)
  1784			return -ENOMEM;
  1785	
  1786		ret = mtd_read(mtd, to, len, &r_retlen, verify_buf);
  1787		if (ret < 0)
  1788			goto err;
  1789	
  1790		if (len != r_retlen) {
  1791			/* We shouldn't see short reads */
  1792			dev_err(dev, "Verify failed, written %zd but only read %zd",
  1793				len, r_retlen);
  1794			ret = -EIO;
  1795			goto err;
  1796		}
  1797	
  1798		if (memcmp(verify_buf, buf, len)) {
  1799			dev_err(dev, "Verify failed, compare mismatch!");
  1800			ret = -EIO;
  1801		}
  1802	
  1803	err:
  1804		devm_kfree(dev, verify_buf);
  1805		return ret;
  1806	}
  1807	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: Verify written data in paranoid mode

[Miquel Raynal]

Hello,

On 08/05/2025 at 20:37:34 +02, Bence Csókás <csokas.bence@prolan.hu> wrote:

> From: Csókás, Bence <csokas.bence@prolan.hu>
>
> Add MTD_PARANOID config option for verifying all written data to prevent
> silent bit errors being undetected, at the cost of some bandwidth overhead.
>
> Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu>
> ---

...

> +int mtd_write_oob(struct mtd_info *mtd, loff_t to,
> +		  struct mtd_oob_ops *ops)
> +{
> +	int ret = _mtd_write_oob(mtd, to, ops);
> +
> +#if IS_ENABLED(CONFIG_MTD_PARANOID)
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = _mtd_verify(mtd, to, ops->retlen, ops->datbuf);
> +#endif // CONFIG_MTD_PARANOID
> +	return ret;
> +}

I'd prefer to extend mtd_write_oob() with a simple

    if (IS_ENABLED(CONFIG_MTD_PARANOID))
            mtd_verify();

It will also likely solve the kernel test robot report.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/