linux-kernel - Re: [PATCH 6/7] mtd: rawnand: sunxi: Update OOB layout to match hardware

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]

Date:   Mon, 2 Jan 2023 10:26:48 -0600
From:   Samuel Holland <samuel@...lland.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Brian Norris <computersforpeace@...il.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mtd@...ts.infradead.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH 6/7] mtd: rawnand: sunxi: Update OOB layout to match
 hardware

Hi Miquèl,

On 1/2/23 03:21, Miquel Raynal wrote:
> Hi Samuel,
> 
> samuel@...lland.org wrote on Thu, 29 Dec 2022 12:15:25 -0600:
> 
>> When using the hardware ECC engine, the OOB data is made available in
>> the NFC_REG_USER_DATA registers, one 32-bit word per ECC step. Any
>> additional bytes are only accessible through raw reads and software
>> descrambling. For efficiency, and to match the vendor driver, ignore
>> these extra bytes.
>>
>> Signed-off-by: Samuel Holland <samuel@...lland.org>
>> ---
>>
>>  drivers/mtd/nand/raw/sunxi_nand.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
>> index 8e873f4fec9a..a3bc9f7f9e5a 100644
>> --- a/drivers/mtd/nand/raw/sunxi_nand.c
>> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
>> @@ -1604,6 +1604,13 @@ static int sunxi_nand_ooblayout_free(struct mtd_info *mtd, int section,
>>  		return 0;
>>  	}
>>  
>> +	/*
>> +	 * The controller does not provide access to OOB bytes
>> +	 * past the end of the ECC data.
>> +	 */
>> +	if (section == ecc->steps && ecc->engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST)
>> +		return -ERANGE;
> 
> Again, I am sorry but I cannot take this change, it would typically
> break jffs2 users (if any?) :(

Considering the bug I fixed in the previous patch, and the fact that
mtd_ooblayout_free() zeroes out the structure before calling the .free
callback, that region was being reported with a length of zero already.
So I don't think anyone could have been using those bytes anyway.

I am looking for a solution here because the ECC/scrambling engine
really provides no way to access these bytes. Reading them requires
turning off the ECC engine, performing another read command, and then
descrambling in software. So we are sort of lying when we claim those
bytes are available with hardware ECC enabled.

If this change cannot be made as-is, is there any way the user could opt
in to the new layout, to get the improved performance?

Regards,
Samuel

>>  	oobregion->offset = section * (ecc->bytes + 4);
>>  
>>  	if (section < ecc->steps)
> 
> 
> Thanks,
> Miquèl