| Message ID | 20180605134950.6605-8-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi, Linus, On 06/05/2018 04:49 PM, Linus Walleij wrote: > Instead of just providing a broad error message about the > chip being unlocked provide details on what is unlocked, > one line per thing that can be locked: data and OTP and > configuration are locked independently. Loose the Failure to lock these zones may permit modification of secret keys, so we don't permit using the device if any of these zones is unlocked. Why do you think it's relevant to indicate which zone is unlocked? Thanks, ta
On Tue, Jun 12, 2018 at 3:25 PM Tudor Ambarus <tudor.ambarus@microchip.com> wrote: > On 06/05/2018 04:49 PM, Linus Walleij wrote: > > Instead of just providing a broad error message about the > > chip being unlocked provide details on what is unlocked, > > one line per thing that can be locked: data and OTP and > > configuration are locked independently. Loose the > > Failure to lock these zones may permit modification of secret keys, so > we don't permit using the device if any of these zones is unlocked. > Why do you think it's relevant to indicate which zone is unlocked? It is always nice for the user to know exactly what the problem is. Instead of "there is some problem with locking" they now know what locking the problem is about: the first, the second or both. Yours, Linus Walleij
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c index fd8149313104..2b6c8bb7bd1c 100644 --- a/drivers/crypto/atmel-ecc.c +++ b/drivers/crypto/atmel-ecc.c @@ -692,8 +692,12 @@ static int device_sanity_check(struct i2c_client *client) * Failure to lock these zones may permit modification of any secret * keys and may lead to other security problems. */ - if (cmd->data[LOCK_CONFIG_IDX] || cmd->data[LOCK_VALUE_IDX]) { - dev_err(&client->dev, "Configuration or Data and OTP zones are unlocked!\n"); + if (cmd->data[RSP_DATA_IDX+3] == 0x55) { + dev_err(&client->dev, "configuration zone is unlocked\n"); + ret = -ENOTSUPP; + } + if (cmd->data[RSP_DATA_IDX+2] == 0x55) { + dev_err(&client->dev, "data and OTP zones are unlocked\n"); ret = -ENOTSUPP; } diff --git a/drivers/crypto/atmel-ecc.h b/drivers/crypto/atmel-ecc.h index 2a378bccc213..988e46507619 100644 --- a/drivers/crypto/atmel-ecc.h +++ b/drivers/crypto/atmel-ecc.h @@ -113,8 +113,6 @@ static const struct { #define CONFIG_ZONE_LKU_4_5 0x13 #define CONFIG_ZONE_LKU_6_7 0x14 #define CONFIG_ZONE_FOOTER 0x15 -#define LOCK_VALUE_IDX (RSP_DATA_IDX + 2) -#define LOCK_CONFIG_IDX (RSP_DATA_IDX + 3) /* * Wake High delay to data communication (microseconds). SDA should be stable
Instead of just providing a broad error message about the chip being unlocked provide details on what is unlocked, one line per thing that can be locked: data and OTP and configuration are locked independently. Loose the overzealous defines. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/crypto/atmel-ecc.c | 8 ++++++-- drivers/crypto/atmel-ecc.h | 2 -- 2 files changed, 6 insertions(+), 4 deletions(-)