linux-kernel - Re: [PATCH] mtd: spinand: fix error read return value

Open Source and information security mailing list archives

Message-ID: <99279437-54a6-c81d-aad2-231009f18cfc@kontron.de>
Date:   Wed, 19 Jun 2019 14:02:14 +0000
From:   Schrempf Frieder <frieder.schrempf@...tron.de>
To:     Boris Brezillon <boris.brezillon@...labora.com>,
        liaoweixiong <liaoweixiong@...winnertech.com>
CC:     Vignesh Raghavendra <vigneshr@...com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Richard Weinberger <richard@....at>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chuanhong Guo <gch981213@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Frieder Schrempf <frieder.schrempf@...eet.de>,
        Clément Péron <peron.clem@...il.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        Brian Norris <computersforpeace@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        "Peter Pan" <peterpandong@...ron.com>
Subject: Re: [PATCH] mtd: spinand: fix error read return value

On 19.06.19 15:46, Boris Brezillon wrote:
> Hi liaoweixiong,
> 
> On Wed, 19 Jun 2019 21:13:24 +0800
> liaoweixiong <liaoweixiong@...winnertech.com> wrote:
> 
>> In function spinand_mtd_read, if the last page to read occurs bitflip,
>> this function will return error value because veriable ret not equal to 0.
> 
> Actually, that's exactly what the MTD core expects (see [1]), so you're
> the one introducing a regression here.

To me it looks like the patch description is somewhat incorrect, but the 
fix itself looks okay, unless I'm getting it wrong.

In case of the last page containing bitflips (ret > 0), 
spinand_mtd_read() will return that number of bitflips for the last 
page. But to me it looks like it should instead return max_bitflips like 
it does when the last page read returns with 0.

>>
>> Signed-off-by: liaoweixiong <liaoweixiong@...winnertech.com>
>> ---
>>   drivers/mtd/nand/spi/core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>> index 556bfdb..6b9388d 100644
>> --- a/drivers/mtd/nand/spi/core.c
>> +++ b/drivers/mtd/nand/spi/core.c
>> @@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
>>   		if (ret == -EBADMSG) {
>>   			ecc_failed = true;
>>   			mtd->ecc_stats.failed++;
>> -			ret = 0;
>>   		} else {
>>   			mtd->ecc_stats.corrected += ret;
>>   			max_bitflips = max_t(unsigned int, max_bitflips, ret);
>>   		}
>>   
>> +		ret = 0;
>>   		ops->retlen += iter.req.datalen;
>>   		ops->oobretlen += iter.req.ooblen;
>>   	}
> 
> [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/mtdcore.c#L1209
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

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