linux-kernel - Re: [RFC PATCH 1/2] mtd: rawnand: handle ONFI revision number field being 0

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, 18 Jun 2018 15:17:30 +0200
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Chris Packham <chris.packham@...iedtelesis.co.nz>,
        dwmw2@...radead.org, computersforpeace@...il.com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Richard Weinberger <richard@....at>,
        Marek Vasut <marek.vasut@...il.com>
Subject: Re: [RFC PATCH 1/2] mtd: rawnand: handle ONFI revision number field
 being 0

On Mon, 18 Jun 2018 15:05:21 +0200
Miquel Raynal <miquel.raynal@...tlin.com> wrote:

> Hi Chris,
> 
> On Mon, 18 Jun 2018 16:52:54 +1200, Chris Packham
> <chris.packham@...iedtelesis.co.nz> wrote:
> 
> > Some Micron NAND chips (MT29F1G08ABAFAWP-ITE:F) report 00 00 for the
> > revision number field of the ONFI parameter page. Rather than rejecting
> > these outright assume ONFI version 1.0 if the revision number is 00 00.
> >   
> 
> Thanks for getting your hands into this.
> 
> > Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> > ---
> > At the moment I haven't qualified this check on anything, I should
> > probably at least include vendor == MICRON.  
> 
> The more I think about it the more I convince myself that this is not
> needed. If the 4 first bytes are "ONFI", then the chip is ONFI...
> 
> Then what you do below is simple and readable and (sadly) probably
> right.

Hm, I'm not entirely convinced considering version = 0 <=> version = 1
is the right thing to do. Clearly, we don't know if other chips are
also broken WRT version field but the actual version might differ.

I'd recommend letting the manufacturer driver fix the param page
instead of guessing what has to been done in the core. That means
adding a new hook to nand_manufacturer_ops (->fixup_onfi_param_page()?)
and calling it just after the CRC has been checked [1].

[1]https://elixir.bootlin.com/linux/v4.18-rc1/source/drivers/mtd/nand/raw/nand_base.c#L5170