linux-kernel - Re: [PATCH v6 4/5] mtd: spi-nor: atmel: Fix unlock

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: <a8ce7d3a61d6f7bd988917291e938954@walle.cc>
Date:   Mon, 30 Nov 2020 15:16:41 +0100
From:   Michael Walle <michael@...le.cc>
To:     Tudor.Ambarus@...rochip.com
Cc:     linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
        boris.brezillon@...labora.com
Subject: Re: [PATCH v6 4/5] mtd: spi-nor: atmel: Fix unlock_all() for
 AT25FS010/040

Am 2020-11-28 09:25, schrieb Tudor.Ambarus@...rochip.com:
> On 11/26/20 10:26 PM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> These flashes have some weird BP bits mapping which aren't supported 
>> in
>> the current locking code. Just add a simple unlock op to unprotect the
>> entire flash array which is needed for legacy behavior.
>> 
>> Signed-off-by: Michael Walle <michael@...le.cc>
>> ---
>> changes since v5
>>  - new patch
>> 
>>  drivers/mtd/spi-nor/atmel.c | 53 
>> +++++++++++++++++++++++++++++++++++--
>>  drivers/mtd/spi-nor/core.c  |  2 +-
>>  drivers/mtd/spi-nor/core.h  |  1 +
>>  3 files changed, 53 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
>> index 49d392c6c8bc..fe6a4653823d 100644
>> --- a/drivers/mtd/spi-nor/atmel.c
>> +++ b/drivers/mtd/spi-nor/atmel.c
>> @@ -8,10 +8,59 @@
>> 
>>  #include "core.h"
>> 
>> +/*
>> + * The Atmel AT25FS010/AT25FS040 parts have some weird configuration 
>> for the
>> + * block protection bits. We don't support them. But legacy behaviour 
>> in linux
>> + * is to unlock the whole flash array on startup. Therefore, we have 
>> to support
>> + * exactly this operation.
>> + */
>> +static int atmel_at25fs_lock(struct spi_nor *nor, loff_t ofs, 
>> uint64_t len)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>> +static int atmel_at25fs_unlock(struct spi_nor *nor, loff_t ofs, 
>> uint64_t len)
>> +{
>> +       /* We only support unlocking the whole flash array */
>> +       if (ofs || len != nor->params->size)
>> +               return -EINVAL;
>> +
>> +       /*
>> +        * Write 0x00 to the status register to try to disable the 
>> write
>> +        * protection. This will fail if SRWD (the datasheet calls it 
>> WPEN) is
>> +        * set. But there is nothing we can do.
>> +        */
> 
> can't we do the same as you did in 5/5?

Sure, but - assuming it is only used for the legacy unlock all operation 
- the
outcome will be the same. It will either keep being locked or all will 
be
unlocked.

That being said, I can also change it to the same as the 
global_unprotect().
I don't have any option on that other than this is simpler.

-michael