linux-kernel - Re: [PATCH] mtd: spi-nor: micron-st: Add n25q064a WP support

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: <CAN8TOE--TYg8r8qCd_HAZ_w2+wRSPF6RUAuK3g-Po5QPVXdRLQ@mail.gmail.com>
Date: Wed, 31 Jul 2024 10:31:50 -0700
From: Brian Norris <computersforpeace@...il.com>
To: Michael Walle <mwalle@...nel.org>
Cc: Tudor Ambarus <tudor.ambarus@...aro.org>, linux-mtd@...ts.infradead.org, 
	Miquel Raynal <miquel.raynal@...tlin.com>, Pratyush Yadav <pratyush@...nel.org>, 
	Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: spi-nor: micron-st: Add n25q064a WP support

On Wed, Jul 31, 2024 at 2:05 AM Michael Walle <mwalle@...nel.org> wrote:
> We really need some kind of dump the relevant registers here. I have
> some very old patch, which dumps the status register, but is has
> it's own quirks. But IMHO we should (maybe additional to the
> functional tests) look at the locking bits in the corresponding
> registers. I.e.
>  flash_lock foobar
>  <verify the status register>
>  flash_unlock foobar
>  <verify the status register>
>  flash_lock barfoo
>  <verify the status register>
>  etc.

I don't actually think that would be a very good test. It would be
testing the implementation more than the functionality. What do you
"verify" in the status register? Would the test just re-implement the
swp.c protection-range logic? And notably, this omits *all* checks
that the protection register actually protects from anything (write,
erase).

Or maybe I'm misinterpreting what you mean.

> Just inferring the correctness from behavior (exercised by writing
> to the flash and verifying it) will lead to errors as it is hard to
> catch all the corner cases.

Why would that lead to errors? It should be relatively easy to:

1. enumerate the supported protection ranges (MEMLOCK / MEMUNLOCK
ioctls on known-likely ranges, looking for EINVAL return codes)
2. iterate through all such ranges; for a given range:
2(a). erase the whole flash
2(b). write the whole flash with a known pattern
2(c). read the whole flash
2(d). ensure that the expected-protected range remains 0xff
2(e). ensure that the expected-unprotected range contains the known pattern

I suppose step #1 can be tough, because the full slate of possible
protection ranges is technically ... enormous. But "likely" ranges are
much fewer, with a few power-of-2 patterns, top/bottom, and maybe some
"both top and bottom" ranges on some flashes? Anyway, like I said in
my other reply, this should take on the order of 60 minutes on some
flashes, which is expensive but not prohibitive.

> From what I remember, flashrom has it's own drivers in userspace,
> no?

Yes, and that's all rather ugly. But it also has a linux_mtd backend
since a few years back:

https://review.coreboot.org/plugins/gitiles/flashrom/+/HEAD/linux_mtd.c

Brian