Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1320959

Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP# is low

Show all headers | View raw

On 28 January 2016 at 14:59, Brian Norris <computersforpeace@gmail.com> wrote:
> Hi Ezequiel,
>
> Thanks for the review.
>
> On Thu, Jan 28, 2016 at 11:36:13AM -0300, Ezequiel Garcia wrote:
>> On 28 January 2016 at 02:51, Brian Norris <computersforpeace@gmail.com> wrote:
>> > Locking the flash is most useful if it provides real hardware security.
>> > Otherwise, it's little more than a software permission bit.
>> >
>> > A reasonable use case that provides real HW security might be like
>> > follows:
>> >
>> > (1) hardware WP# is deasserted
>> > (2) program flash
>> > (3) flash range is protected via status register
>> > (4) hardware WP# is asserted
>> > (5) flash protection range can no longer be changed, until WP# is
>> >     deasserted
>> >
>> > In this way, flash protection is co-owned by hardware and software.
>> >
>> > Now, one would expect to be able to perform step (3) with
>> > ioctl(MEMLOCK), except that the spi-nor driver does not set the Status
>> > Register Protect bit (a.k.a. Status Register Write Disable (SRWD)), so
>> > even though the range is now locked, it does not satisfy step (5) -- it
>> > can still be changed by a call to ioctl(MEMUNLOCK).
>> >
>> > So, let's enable status register protection after the first lock
>> > command.
>> >
>> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>> > ---
>> >  drivers/mtd/spi-nor/spi-nor.c |    3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> > index 3a08aa53c171..46da6bb706fa 100644
>> > --- a/drivers/mtd/spi-nor/spi-nor.c
>> > +++ b/drivers/mtd/spi-nor/spi-nor.c
>> > @@ -518,6 +518,9 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> >
>> >         status_new = (status_old & ~mask) | val;
>> >
>> > +       /* Disallow further writes if WP pin is asserted */
>> > +       status_new |= SR_SRWD;
>> > +
>>
>> No need to clear SR_SRWD in stm_unlock?
>
> Good point.
>
> I actually had thought about that earlier, but I didn't come up with a
> great plan, and then I forgot about it when I was preparing this RFC. I
> don't think we want *all* "unlock" operations to unprotect the status
> register. What if we had the whole flash locked, and we're just calling
> unlock on the top half, with the intention of leaving the bottom half
> protected still?
>

Right.

> So, maybe we want to clear SR_SRWD only when we unlock the *entire*
> flash? What do you think?
>

How about this:

1) ioctl(MEMLOCK) the entire flash (SR_SRWD is set)
2) ioctl(MEMUNLOCK) partially (SW_SRWD keeps set)
3) ioctl(MEMLOCK) the entire flash again

Not sure this use case make sense, but would (3)  be allowed given
SW_SRWD is set?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/8] mtd: spi-nor: locking fixes and updates Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  [PATCH 8/8] mtd: spi-nor: support lock/unlock for a few Winbond chips Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  [PATCH 2/8] mtd: spi-nor: guard against underflows in stm_is_locked_sr Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP# is low Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
    Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP# is low Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2016-01-28 15:40 +0100
      Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP#  is low Brian Norris <computersforpeace@gmail.com> - 2016-01-28 19:00 +0100
        Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP# is low Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2016-01-28 20:30 +0100
          Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP#  is low Brian Norris <computersforpeace@gmail.com> - 2016-01-28 20:50 +0100
            Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP# is low Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2016-01-29 14:30 +0100
              Re: [PATCH 4/8] mtd: spi-nor: disallow further writes to SR if WP#  is low Brian Norris <computersforpeace@gmail.com> - 2016-01-29 20:30 +0100
  [PATCH 5/8] mtd: spi-nor: use BIT() for flash_info flags Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  [PATCH 6/8] mtd: spi-nor: add SPI_NOR_HAS_LOCK flag Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  [PATCH 1/8] mtd: spi-nor: wait for SR_WIP to clear on initial unlock Brian Norris <computersforpeace@gmail.com> - 2016-01-28 07:00 +0100
  Re: [PATCH 0/8] mtd: spi-nor: locking fixes and updates Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2016-01-28 15:50 +0100

csiph-web