Re: [PATCH 8/9] crypto: atmel-ecc: Detail what is unlocked

[Linus Walleij]

On Tue, Jun 12, 2018 at 3:25 PM Tudor Ambarus
 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

Re: [PATCH 8/9] crypto: atmel-ecc: Detail what is unlocked

[Tudor Ambarus]

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

[PATCH 8/9] crypto: atmel-ecc: Detail what is unlocked

[Linus Walleij]

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 
---
 drivers/crypto/atmel-ecc.c | 8 ++--
 drivers/crypto/atmel-ecc.h | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

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(>dev, "Configuration or Data and OTP zones are 
unlocked!\n");
+   if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+   dev_err(>dev, "configuration zone is unlocked\n");
+   ret = -ENOTSUPP;
+   }
+   if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+   dev_err(>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_50x13
 #define CONFIG_ZONE_LKU_6_70x14
 #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
-- 
2.17.0