Re: [PATCH v2 1/3] mtd: spi-nor: always respect write-protect input

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 20/03/2019 19:56, Yong Qin wrote:
Hi Jonas, Tudor,

I think Tudor described is a reasonable use case scenario.

BPNV bit default value is 0, which means BP0-2 bits are default Non-Volatile. In this case, BPNV bit does not need to be set. After the flash chip mounted on PCB board with WP# grounded, all need to do are: Program data into flash --> Set BP0-2 to 1 --> Set SRWD to 1, then both SRWD bit and BP0-2 bits are locked-down, unless disconnecting WP# from grounding. Reset will not change SRWD bit and BP0-2 bits since they all are Non-Volatile.

OK, my own use case is causing me to get confused over the volatility of these bits.

The use case I have is the following:
a)  The flash must be write-protected at power on
b)  The flash may be made writable by an authorized actor

So the idea is to do this:
i)  Set BPNV to 1 so that BP0-2 get reset to 1 at power on
ii)  WP# is asserted at power on (effectively grounded)
iii)  Authorized actor invokes action to de-assert WP#
iv)  BP0-2 bits can now be modified freely and flash is writable
v) Authorized actor may invoke action to assert WP# but if that doesn't happen and the system gets restarted at least the flash will not be writable when it is restarted

Control of the BP bits in Linux is via stm_lock and stm_unlock. These functions also set and clear SRWD. This breaks the above use case because when the device is unlocked, SRWD is cleared, and then device is not protected after a power cycle.

Suggestions? Can we separate the manipulation of SRWD from the manipulation of the BP bits without breaking existing users?

/Jonas


Best Regards,
Yong

-----Original Message-----
From: Jonas Bonn <jonas@xxxxxxxxxxx>
Sent: Wednesday, March 20, 2019 3:40 AM
To: Tudor.Ambarus@xxxxxxxxxxxxx; Yong Qin <Yong.Qin@xxxxxxxxxxx>; James Tomasetta <James.Tomasetta@xxxxxxxxxxx>
Cc: bbrezillon@xxxxxxxxxx; richard@xxxxxx; marek.vasut@xxxxxxxxx; linux-mtd@xxxxxxxxxxxxxxxxxxx; computersforpeace@xxxxxxxxx; dwmw2@xxxxxxxxxxxxx
Subject: Re: [PATCH v2 1/3] mtd: spi-nor: always respect write-protect input



On 20/03/2019 08:33, Tudor.Ambarus@xxxxxxxxxxxxx wrote:


On 03/20/2019 09:06 AM, Jonas Bonn wrote:
External E-Mail



On 20/03/2019 07:33, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
Jonas,

On 03/19/2019 09:13 AM, Jonas Bonn wrote:

On 19/03/2019 07:59, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
Jonas, Yong,

On 03/12/2019 09:27 PM, Yong Qin wrote:
Hi Tudor,

Good question.

The WP# function is not available when the Quad mode is enabled (CR[1]=1). The WP# function is replaced by IO2 for input and output during Quad mode. With that said, if CR[1] is set to 1 (Quad mode enabled), the registers will not be protected.

Technically default SRWD bit can be set as either 0 or 1. However, as most customers (applications) don't use this feature to protect registers, the default SRWD bit set to 0 might be a better choice, and reserve the option to change to 1 for the applications do need this feature.

I think I found the reason why SRWD bit is configurable, and
disabled by
default: => to allow the installation of the flash in a system
with a grounded WP# pin while still enabling write to the BP bits.

I think this is bogus.  Why would you ground the SRWD pin?  That's a design error.

I've talked with Microchip hardware team, we have this feature on sst26 flashes too.

Grounding WP would not necessarily be a design error. The intention
is that some users might choose to populate the Flash chip onto
their PCB, program the memory in-circuit, and then lock down the write protection to use the chip like a ROM.
Grounding WP allows them to do this. Since SRWD is set to '0' from
the factory, so users can program the memory, set the block
protection (BP) bits to protect the memory, and then set the SRWD
bit to enable the grounded WP input and prevent changing the BP bits.

Again, this is bogus.  The SRWD bit is non-volatile so just resetting the flash clears the bit and the BP bits can be changed.  This does not magically turn the flash into a ROM.  Grounded or not, the WP# is NOT respected until the SRWD bit is set; what you've described is yet another reason to default the SRWD to set.

SRWD is a non-volatile bit: default at power-up will be the setting
prior to power-down.

Ah, right you are! :)

By grounding the WP# pin, you are effectively turning the SRWD bit into an OTP bit.  Note that what you described above also requires setting the BPNV bit, otherwise protection is irrevocably reset to "unprotected"
at reset.  The BPNV bit cannot be set from Linux so whoever is using the "grounded pin" method must be programming the board in-factory from another system than Linux.  Therefore it's OK for Linux to default SRWD to asserted, irregardless of the state of WP#.

/Jonas




Cheers,
ta


With the BPNV bit (see patch 3 that I posted in the v2 series), at least the flash can be made to start up write-protected; however, there is nothing preventing modification of the PROTection bits until SRWD is set AND WP# is asserted (through grounding or otherwise).  Linux currently defaults SRWD to cleared which is "unprotected", irregardless of the state of WP#.


The grounded WP pin + SRWD bit method is an older, legacy approach
to the problem. We don't know how many users actually ground the WP
pin in this manner, but there are probably some users out there who do.

If they do so, they are fooling themselves... or they are running a
patched kernel! :)

/Jonas





Cheers,
ta


/Jonas


Jonas, Yong, what do you think?

Cheers,
ta


Thanks,
Yong

-----Original Message-----
From: Tudor.Ambarus@xxxxxxxxxxxxx <Tudor.Ambarus@xxxxxxxxxxxxx>
Sent: Tuesday, March 12, 2019 5:30 AM
To: Yong Qin <Yong.Qin@xxxxxxxxxxx>; jonas@xxxxxxxxxxx; James
Tomasetta <James.Tomasetta@xxxxxxxxxxx>
Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; bbrezillon@xxxxxxxxxx;
richard@xxxxxx; marek.vasut@xxxxxxxxx;
computersforpeace@xxxxxxxxx; dwmw2@xxxxxxxxxxxxx
Subject: Re: [PATCH v2 1/3] mtd: spi-nor: always respect
write-protect input

Hi, Yong,

Thank you for the explanation. There are still few things to clarify.

On 03/11/2019 10:14 PM, Yong Qin wrote:
SRWD bit (along with WP#) provides a way to protect Status and Configuration Registers from been modified unintendedly or by a malicious actor.

By default, SRWD bit is 0, which means no protection on registers alternations. Registers can be modified easily by WRR command. (this is most of the application use cases).

If set SRWD bit to 1, then when WP# is driven low during WRR command, WRR command will be ignored and Registers can't be modified. This provides a way to protect Registers, meanwhile still reserve the capability to modify Registers when necessary by driving WP# to high during WRR command.

Does the SRWD bit protect the Status and Configuration Register bits even when in Quad Mode? WP# function is not available in Quad mode. How can one release this protection when in Quad Mode and SRWD set to 1?

If SRWD bit is ignored in Quad Mode, then why didn't Cypress enable Status and Configuration Register bits protection by default? I.e., remove SRWD bit from SR1, make BIT(7) a NOP, and consider the Status and Configuration Register bits protection enabled by default when not in Quad Mode.

Cheers,
ta

This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/



[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux