linux-kernel - Re: [PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme

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: <35446d76-5712-c51f-e1ab-f81a53b1a4aa@microchip.com>
Date:   Wed, 13 Feb 2019 16:23:37 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <palmer@...ive.com>, <marek.vasut@...il.com>
CC:     <wesley@...ive.com>, <richard@....at>,
        <linux-kernel@...r.kernel.org>, <bbrezillon@...nel.org>,
        <linux-mtd@...ts.infradead.org>, <paul.walmsley@...ive.com>,
        <computersforpeace@...il.com>, <dwmw2@...radead.org>
Subject: Re: [PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking
 scheme

Hi, Wesley, Palmer,

On 08/07/2018 10:40 PM, Palmer Dabbelt wrote:

First of all, thanks for the patience!

> From: "Wesley W. Terpstra" <wesley@...ive.com>
> 
> ISSI uses a non-standard scheme to control block protection, with bit 5
> of the status registerr controlling an additional block protection bit.

Indeed, issi's block protection (BP3, BP2, BP1, BP0) bits are used in a
different way than how are used in stm_lock/unlock, even with the 4bit block
protection on its way (see [1]).

> This patch disables all the block protection bits whenever an ISSI chip
> is seen.

IS25WP256's datasheet says that "The default value of the BP0, BP1, BP2, BP3,
QE, and SRWD bits were set to “0”
 at factory.", which means that all memory blocks come unprotected (unlocked) by
default.

I see your patch as an extra safety measure for people that somehow
unfortunately set these block protection bits, so that they not end up with
locked blocks. Instead of adding this extra safety check/set, I would suggest to
actually add support for the issi's block protection scheme. In a perfect world,
this would fit in a per-manufacturer hook after the per-manufacturer split up
code will be integrated (see [2]).

> 
> We might also want to trigger an error when writing SR_TB to these
> chips, as it aliases with this extra protection bit in the status
> register.  It looks like that's always conditional on SNOR_F_HAS_SR_TB,
> so at least what's there is safe.

This problem will vanish when you'll have the issi's protection scheme implemented.

Cheers,
ta

[1] https://patchwork.ozlabs.org/patch/1011820/
[2] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=80353