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

Message ID 20180605134950.6605-8-linus.walleij@linaro.org
State Superseded
Headers show
Series
  • [1/9] crypto: atmel-ecc: Make available for other platforms
Related show

Commit Message

Linus Walleij June 5, 2018, 1:49 p.m.
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(-)

-- 
2.17.0

Comments

Tudor Ambarus June 12, 2018, 1:25 p.m. | #1
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
Linus Walleij June 28, 2018, 9:28 a.m. | #2
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

Patch

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