linux-kernel - Re: [PATCH v1 4/5] mtd: rawnand: meson: clear OOB buffer before read

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: <fe03919a-3468-b79e-5971-1f7671684ee9@amlogic.com>
Date:   Wed, 12 Apr 2023 19:36:30 +0800
From:   Liang Yang <liang.yang@...ogic.com>
To:     Arseniy Krasnov <avkrasnov@...rdevices.ru>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     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>,
        Jianxin Pan <jianxin.pan@...ogic.com>,
        Yixun Lan <yixun.lan@...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 v1 4/5] mtd: rawnand: meson: clear OOB buffer before read

Hi,

On 2023/4/12 18:51, Liang Yang wrote:
>>>>>> diff --git a/drivers/mtd/nand/raw/meson_nand.c 
>>>>>> b/drivers/mtd/nand/raw/meson_nand.c
>>>>>> index f84a10238e4d..f2f2472cb511 100644
>>>>>> --- a/drivers/mtd/nand/raw/meson_nand.c
>>>>>> +++ b/drivers/mtd/nand/raw/meson_nand.c
>>>>>> @@ -858,9 +858,12 @@ static int meson_nfc_read_page_sub(struct 
>>>>>> nand_chip *nand,
>>>>>>   static int meson_nfc_read_page_raw(struct nand_chip *nand, u8 *buf,
>>>>>>                      int oob_required, int page)
>>>>>>   {
>>>>>> +    struct mtd_info *mtd = nand_to_mtd(nand);
>>>>>>       u8 *oob_buf = nand->oob_poi;
>>>>>>       int ret;
>>>>>> +    memset(oob_buf, 0, mtd->oobsize);
>>>>>
>>>>> I'm surprised raw reads do not read the entire OOB?
>>>>
>>>> Yes! Seems in case of raw access (what i see in this driver) number 
>>>> of OOB bytes read
>>>> still depends on ECC parameters: for each portion of data covered 
>>>> with ECC code we can
>>>> read it's ECC code and "user bytes" from OOB - it is what i see by 
>>>> dumping DMA buffer by
>>>> printk(). For example I'm working with 2K NAND pages, each page has 
>>>> 2 x 1K ECC blocks.
>>>> For each ECC block I have 16 OOB bytes which I can access by 
>>>> read/write. Each 16 bytes
>>>> contains 2 bytes of user's data and 14 bytes ECC codes. So when I 
>>>> read page in raw mode
>>>> controller returns 32 bytes (2 x (2 + 14)) of OOB. While OOB is 
>>>> reported as 64 bytes.
>>>
>>> In all modes, when you read OOB, you should get the full OOB. The fact
>>> that ECC correction is enabled or disabled does not matter. If the NAND
>>> features OOB sections of 64 bytes, you should get the 64 bytes.
>>>
>>> What happens sometimes, is that some of the bytes are not protected
>>> against bitflips, but the policy is to return the full buffer.
>>
>> Ok, so to clarify case for this NAND controller:
>> 1) In both ECC and raw modes i need to return the same raw OOB data 
>> (e.g. user bytes
>>     + ECC codes)?
>> 2) If I have access to only 32 bytes of OOB (in case above), I must 
>> report that size
>>     of OOB is only 32 bytes during initialization?
>>
>> Thanks, Arseniy
> 
> Yes. it should return all the OOB data. i make a mistake on raw read and 
> there is wrong code in meson_nfc_read_page_raw().
>      meson_nfc_get_data_oob(nand, buf, oob_buf);
> changed to:
>      if (oob_required)
>          memcpy(oob_buf, buf + mtd->writesize, mtd->oobsize)

Sorry, please ignore this. the previous code is right.

the controller changes the layout of one page; the physical layout is 
2048 main data + 64 oob data. after writing into NAND page, it is stored
like this: 1024 main data + 2 user bytes + 14 ECC parity bytes + 1024 
main data + 2 user bytes + 14 ECC parity bytes. so that is right we only 
get 4 user bytes and 28 ECC parity bytes, total 32 bytes. that is the 
behavior of the controller that transferring one ECC page(1KB) brings 
back only 2 user bytes.

because layout is changed by controller, so go back to the function. 
meson_nfc_get_data_oob(nand, buf, oob_buf) try to get the right user and 
ecc parity bytes from the right pos. after that, the other oob bytes is 
not reading from NAND flash.


-- 
Thanks,
Liang