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: <20230530102143.6b2a199a@xps-13>
Date:   Tue, 30 May 2023 10:21:43 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Arseniy Krasnov <avkrasnov@...rdevices.ru>
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 Arseniy,

avkrasnov@...rdevices.ru wrote on Tue, 30 May 2023 11:09:10 +0300:

> 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?

Yes.

The problem you face is due to jffs2 using free OOB bytes to store some
data. If this data is in the protected area -> BOOM.

If another application wants to use all the bytes and writes them all
in the same PROGRAM operation it's fine.

Jffs2 accesses the free area through the OOB layouts only, so just
tweaking the OOB layouts should work.

Thanks,
Miquèl