linux-kernel - Re: mtd: nand: fsl_ifc: fix handing of bit flips in erased pages

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-next>] [day] [month] [year] [list]

Message-ID: <5f665145ea304d3a9e1ecb39c51a951e@svr-chch-ex1.atlnz.lc>
Date:   Thu, 25 May 2017 23:33:43 +0000
From:   Darwin Dingel <Darwin.Dingel@...iedtelesis.co.nz>
To:     Pavel Machek <pavel@....cz>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>
CC:     "richard@....at" <richard@....at>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>,
        "cyrille.pitchen@...el.com" <cyrille.pitchen@...el.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "mark.marshall@...cronenergy.com" <mark.marshall@...cronenergy.com>,
        "b44839@...escale.com" <b44839@...escale.com>,
        "prabhakar@...escale.com" <prabhakar@...escale.com>
Subject: Re: mtd: nand: fsl_ifc: fix handing of bit flips in erased pages

Hi,

We are also having the same problem where the IFC (nand flash) was 
reporting ECC uncorrectable errors on single bitflips with erased pages. 
Applying this patch with some minor modifications seems to solve our 
issue. We are still doing more testing but recent results looks promising.

Our kernel is 4.4.6 so we have to modify it a bit to fit the old ECC 
layout structure. We just have a few comments about the patch:

 > -				if (!is_blank(mtd, bufnum))
 > -					ctrl->nand_stat |=
 > -						IFC_NAND_EVTER_STAT_ECCER;
 > -				break;
 > +				ctrl->nand_stat |= IFC_NAND_EVTER_STAT_ECCER;

Added 'error = 0' after setting the flag since no error was actually 
corrected.

 > -	if (ctrl->nand_stat & IFC_NAND_EVTER_STAT_ECCER)
 > -		dev_err(priv->dev, "NAND Flash ECC Uncorrectable Error\n");
 > -
 >  	if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
 >  		mtd->ecc_stats.failed++;
 > +
 > +	if (ctrl->nand_stat & IFC_NAND_EVTER_STAT_ECCER) {
 > +		int res;
 > +
 > +		if (!oob_required)
 > +			fsl_ifc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
 > +
 > +		res = check_erased_page(chip, buf);
 > +		return res;
 > +	}

We have to do the check IFC_NAND_EVTER_STAT_ECCER first because the 
condition (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) will never be 
true since IFC always sets IFC_NAND_EVTER_STAT_ECCER on empty pages. 
Incrementing failed stats first before doing check_erased_page() makes 
nand_read() report ECC error all time.

Our exact modification was:
	if (ctrl->nand_stat & IFC_NAND_EVTER_STAT_ECCER) {
		if (!oob_required)
			fsl_ifc_read_buf(mtd, chip->oob_poi, mtd->oobsize);

		return check_erased_page(chip, buf);
	}

	if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
		mtd->ecc_stats.failed++;

Because check_erased_page() will be updating the failed stat anyway.

Cheers,
Darwin