linux-kernel - Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <95bf5e07-7f03-efb0-da9b-cb50999c2524@huawei.com>
Date: Tue, 2 Sep 2025 19:41:06 +0800
From: Zhihao Cheng <chengzhihao1@...wei.com>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>, <miquel.raynal@...tlin.com>,
	<richard@....at>, <vigneshr@...com>
CC: <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<yi.zhang@...wei.com>, <yangerkun@...wei.com>
Subject: Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already
 known bad

在 2025/9/2 17:27, Wang Zhaolong 写道:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
> 
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
> 
> This keeps the logic centralized in mtdcore without requiring driver
> changes.
> 
> Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
> Signed-off-by: Wang Zhaolong <wangzhaolong@...weicloud.com>
> ---
> 
> V2:
> - Checks old state when _block_isbad exists and bails out early if already bad
> 
>   drivers/mtd/mtdcore.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5ba9a741f5ac..096a3c94670f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
>   EXPORT_SYMBOL_GPL(mtd_block_isbad);
>   
>   int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>   {
>   	struct mtd_info *master = mtd_get_master(mtd);
> +	loff_t moffs;
>   	int ret;
>   
>   	if (!master->_block_markbad)
>   		return -EOPNOTSUPP;
>   	if (ofs < 0 || ofs >= mtd->size)
> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>   		return -EROFS;
>   
>   	if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>   		ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>   
> -	ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> +	moffs = mtd_get_master_ofs(mtd, ofs);
> +
> +	if (master->_block_isbad) {
> +		ret = master->_block_isbad(master, moffs);
> +		if (ret > 0)
> +			return 0;

Hi, Miquèl.
Here, should we keep the same logic with the lower level(eg. 
nand_block_markbad, onenand_block_markbad) when 'ret < 0' is returned by 
master->_block_isbad. Many specific nand drivers(markbad) return the 
negative code when 'isbad' fails.

> +	}
> +
> +	ret = master->_block_markbad(master, moffs);
>   	if (ret)
>   		return ret;
>   
>   	while (mtd->parent) {
>   		mtd->ecc_stats.badblocks++;
>