linux-kernel - [PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20211216083418.13512-6-kernel@kempniu.pl>
Date:   Thu, 16 Dec 2021 09:34:18 +0100
From:   Michał Kępień <kernel@...pniu.pl>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>
Cc:     Boris Brezillon <boris.brezillon@...labora.com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2 5/5] mtdchar: extend MEMREAD ioctl to return ECC statistics

While returning -EUCLEAN or -EBADMSG is a valid signal for user space
that the ECC algorithm detected errors during a read operation, that
signal is not granular enough to cover all use cases.  For example,
knowing the maximum number of bitflips detected in a single ECC step
during a read operation performed on a given page may be useful when
dealing with an MTD partition whose ECC layout varies across pages (e.g.
a partition consisting of a bootloader area using a "custom" ECC layout
followed by data pages using a "standard" ECC layout).

Extend struct mtd_read_req with a newly introduced struct
mtd_read_req_ecc_stats and set the fields of that structure in
mtdchar_read_ioctl().  Use ECC statistics to ensure the error code
returned by the MEMREAD ioctl accounts for all ECC errors detected
during the read operation.

Link: https://www.infradead.org/pipermail/linux-mtd/2016-April/067085.html

Suggested-by: Boris Brezillon <boris.brezillon@...labora.com>
Signed-off-by: Michał Kępień <kernel@...pniu.pl>
---
 drivers/mtd/mtdchar.c      | 26 ++++++++++++++++++++++++++
 include/uapi/mtd/mtd-abi.h | 21 +++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 6ad59c9eed2f..68cc91d82a5d 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -714,6 +714,10 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
 	if (!usr_oob)
 		req.ooblen = 0;
 
+	req.ecc_stats.uncorrectable_errors = 0;
+	req.ecc_stats.corrected_bitflips = 0;
+	req.ecc_stats.max_bitflips = 0;
+
 	if (req.start + req.len > mtd->size) {
 		ret = -EINVAL;
 		goto out;
@@ -738,12 +742,14 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
 	}
 
 	while (req.len > 0 || (!usr_data && req.ooblen > 0)) {
+		struct mtd_req_stats stats;
 		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,
+			.stats = &stats,
 		};
 
 		/*
@@ -757,6 +763,13 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
 			ops.len -= mtd_mod_by_ws(req.start + ops.len, mtd);
 
 		ret = mtd_read_oob(mtd, (loff_t)req.start, &ops);
+
+		req.ecc_stats.uncorrectable_errors +=
+			stats.uncorrectable_errors;
+		req.ecc_stats.corrected_bitflips += stats.corrected_bitflips;
+		req.ecc_stats.max_bitflips =
+			max(req.ecc_stats.max_bitflips, stats.max_bitflips);
+
 		if (ret && !mtd_is_bitflip_or_eccerr(ret))
 			break;
 
@@ -774,6 +787,19 @@ static int mtdchar_read_ioctl(struct mtd_info *mtd,
 		usr_oob += ops.oobretlen;
 	}
 
+	/*
+	 * As multiple iterations of the above loop (and therefore multiple
+	 * mtd_read_oob() calls) may be necessary to complete the read request,
+	 * adjust the final return code to ensure it accounts for all detected
+	 * ECC errors.
+	 */
+	if (!ret || mtd_is_bitflip(ret)) {
+		if (req.ecc_stats.uncorrectable_errors > 0)
+			ret = -EBADMSG;
+		else if (req.ecc_stats.corrected_bitflips > 0)
+			ret = -EUCLEAN;
+	}
+
 out:
 	req.len = orig_len - req.len;
 	req.ooblen = orig_ooblen - req.ooblen;
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index 337e6e597fad..bc68f266c174 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -91,6 +91,25 @@ struct mtd_write_req {
 	__u8 padding[7];
 };
 
+/**
+ * struct mtd_read_req_ecc_stats - ECC statistics for a read operation
+ *
+ * @uncorrectable_errors: the number of uncorrectable errors that happened
+ *			  during the read operation
+ * @corrected_bitflips: the number of bitflips corrected during the read
+ *			operation
+ * @max_bitflips: the maximum number of bitflips detected in any single ECC
+ *		  step for the data read during the operation; this information
+ *		  can be used to decide whether the data stored in a specific
+ *		  region of the MTD device should be moved somewhere else to
+ *		  avoid data loss.
+ */
+struct mtd_read_req_ecc_stats {
+	__u32 uncorrectable_errors;
+	__u32 corrected_bitflips;
+	__u32 max_bitflips;
+};
+
 /**
  * struct mtd_read_req - data structure for requesting a read operation
  *
@@ -101,6 +120,7 @@ struct mtd_write_req {
  * @usr_oob:	user-provided OOB buffer
  * @mode:	MTD mode (see "MTD operation modes")
  * @padding:	reserved, must be set to 0
+ * @ecc_stats:	ECC statistics for the read operation
  *
  * This structure supports ioctl(MEMREAD) operations, allowing data and/or OOB
  * reads in various modes. To read from OOB-only, set @usr_data == NULL, and to
@@ -115,6 +135,7 @@ struct mtd_read_req {
 	__u64 usr_oob;
 	__u8 mode;
 	__u8 padding[7];
+	struct mtd_read_req_ecc_stats ecc_stats;
 };
 
 #define MTD_ABSENT		0
-- 
2.34.1