linux-kernel - Re: [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC area

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: <1f4c90c4-e436-c53f-bb6f-416db374ae52@sberdevices.ru>
Date:   Tue, 30 May 2023 11:09:10 +0300
From:   Arseniy Krasnov <avkrasnov@...rdevices.ru>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
CC:     Liang Yang <liang.yang@...ogic.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Yixun Lan <yixun.lan@...ogic.com>,
        Jianxin Pan <jianxin.pan@...ogic.com>, <oxffffaa@...il.com>,
        <kernel@...rdevices.ru>, <linux-mtd@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-amlogic@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/5] mtd: rawnand: meson: move OOB to non-protected ECC
 area

Hi Miquel,

On 30.05.2023 10:44, Miquel Raynal wrote:
> Hi Arseniy,
> 
>>>>>> -static void meson_nfc_get_user_byte(struct nand_chip *nand, u8 *oob_buf)
>>>>>> -{
>>>>>> -	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>>>>>> -	__le64 *info;
>>>>>> -	int i, count;
>>>>>> +	int i;
>>>>>>  
>>>>>> -	for (i = 0, count = 0; i < nand->ecc.steps; i++, count += 2) {
>>>>>> +	for (i = 0; i < nand->ecc.steps; i++) {
>>>>>>  		info = &meson_chip->info_buf[i];
>>>>>> -		oob_buf[count] = *info;
>>>>>> -		oob_buf[count + 1] = *info >> 8;
>>>>>> +		/* Always ignore user bytes programming. */    
>>>>>
>>>>> Why?    
>>>>
>>>> I think comment message is wrong a little bit. Here "user bytes" are
>>>> user bytes protected by ECC (e.g. location of these bytes differs from new
>>>> OOB layout introduced by this patch). During page write this hardware
>>>> always writes these bytes along with data. But, new OOB layout always ignores
>>>> these 4 bytes, so set them to 0xFF always.  
>>>
>>> When performing page reads/writes, you need to take the data as it's
>>> been provided. You may move the data around in the buffer provided to
>>> the controller, so that it get the ECC data at the right location, and
>>> you need of course to reorganize the data when reading as well, so that
>>> the user sees XkiB of data + YB of OOB. That's all you need to do in
>>> these helpers.
>>>   
>>
>> I think there is some misunderstanding about these "user bytes" above: there are 4
>> bytes which this NAND controller always writes to page in ECC mode - it was free OOB
>> bytes covered by ECC. Controller grabs values from DMA buffer (second DMA buffer which
>> doesn't contains page data) and writes it along with data and ECC codes. Idea of this
>> change is to always suppress this write by setting them to 0xFF (may be there is some
>> command option to not write it, but I don't have doc), because all of them (4 bytes)
>> become unavailable to reader/writer.
> 
> At the NAND controller level, I would rather avoid doing things like
> that.
> 
> I believe you can just update the ooblayout so that protected OOB bytes
> are not exposed to the user as free bytes. Then your buffers should
> already contain 0xffffff at the problematic location.

So Your idea is to continue fill DMA buffer (for these 4 bytes) from provided OOB buffer,
relying on that as these bytes are unused, they will be 0xFF in OOB buffer so we get the same result?

Thanks, Arseniy

> 
> Thanks,
> Miquèl