linux-kernel - Re: [PATCH v2 7/8] mtd: spi-nor: add TB (Top/Bottom) protect 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]

Date:	Mon, 29 Feb 2016 17:35:02 -0300
From:	Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:	Brian Norris <computersforpeace@...il.com>
Cc:	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Rafał Miłecki <zajec5@...il.com>,
	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Bayi Cheng <bayi.cheng@...iatek.com>,
	Marek Vasut <marex@...x.de>,
	Daniel Kurtz <djkurtz@...omium.org>
Subject: Re: [PATCH v2 7/8] mtd: spi-nor: add TB (Top/Bottom) protect support

Hi Brian,

On 29 January 2016 at 16:25, Brian Norris <computersforpeace@...il.com> wrote:
> Some flash support a bit in the status register that inverts protection
> so that it applies to the bottom of the flash, not the top. This yields
> additions to the protection range table, as noted in the comments.
>
> Because this feature is not universal to all flash that support
> lock/unlock, control it via a new flag.
>
> Signed-off-by: Brian Norris <computersforpeace@...il.com>
> ---
> v2:
>  * Rewrite the bounds checking for top/bottom support, since there were some
>    bad corner cases. Now lock/unlock are more symmetric.
>
>  drivers/mtd/spi-nor/spi-nor.c | 70 ++++++++++++++++++++++++++++++++++++++-----
>  include/linux/mtd/spi-nor.h   |  2 ++
>  2 files changed, 65 insertions(+), 7 deletions(-)
>
[..]
> @@ -476,12 +484,14 @@ static int stm_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len,
>
>  /*
>   * Lock a region of the flash. Compatible with ST Micro and similar flash.
> - * Supports only the block protection bits BP{0,1,2} in the status register
> + * Supports the block protection bits BP{0,1,2} in the status register
>   * (SR). Does not support these features found in newer SR bitfields:
> - *   - TB: top/bottom protect - only handle TB=0 (top protect)
>   *   - SEC: sector/block protect - only handle SEC=0 (block protect)

While reviewing and testing this patchset, I realised that *no* Micron device
define BIT(6) as SEC (sector/block) bit. Instead, it's used as BP3, to extend
the region defined by BP0-BP2.

I've checked the following:

  N25Q256A
  N25Q128A
  N25Q064A
  N25Q032A
  N25Q016A
  M25Pxx

So I believe we need to separate stm_{lock,unlock), from
winbond_{lock,unlock}. We might want to explicitly mark devices that
currently support locking with the new _HAS_LOCK flag.

Also, I wonder if we can really separate based on vendor, or if we'll need
more flags to distinguish the lock implementation per device.

Of course, all the devices that define a BP3 are broken with respect to flash
locking. I can try to cook some patches for this, once we are decided on how
to do it.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar