[9/9,v2] crypto: atmel-ecc: Break out lock check helper

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

Commit Message

Linus Walleij June 28, 2018, 1:07 p.m.
This breaks out a lock status checker to be used with further
refactorings.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
ChangeLog v1->v2:
- Rebased
---
 drivers/crypto/atmel-ecc.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

-- 
2.17.0

Comments

Tudor Ambarus July 2, 2018, 3:23 p.m. | #1
On 06/28/2018 04:07 PM, Linus Walleij wrote:
> This breaks out a lock status checker to be used with further

> refactorings.

> 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

> ChangeLog v1->v2:

> - Rebased

> ---

>  drivers/crypto/atmel-ecc.c | 38 ++++++++++++++++++++++++++++++++++----

>  1 file changed, 34 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c

> index 0dfea6eadc56..549ec7190287 100644

> --- a/drivers/crypto/atmel-ecc.c

> +++ b/drivers/crypto/atmel-ecc.c

> @@ -666,7 +666,9 @@ static int atmel_ecc_get_serial(struct i2c_client *client)

>  	return ret;

>  }

>  

> -static int device_sanity_check(struct i2c_client *client)

> +static int atmel_ecc_get_lock_status(struct i2c_client *client,

> +				     bool *config_is_locked,

> +				     bool *otp_data_is_locked)

>  {

>  	struct atmel_ecc_cmd *cmd;

>  	int ret;

> @@ -680,21 +682,49 @@ static int device_sanity_check(struct i2c_client *client)

>  	ret = atmel_ecc_send_receive(client, cmd);

>  	if (ret) {

>  		dev_err(&client->dev,

> -			"failed to send ECC init command\n");

> +			"failed to send config read command\n");

>  		goto free_cmd;

>  	}

>  

> +	/* According to datasheet anything else than 0x55 means "locked" */

> +	if (cmd->data[RSP_DATA_IDX+3] == 0x55)

> +		*config_is_locked = false;

> +	else

> +		*config_is_locked = true;

> +

> +	if (cmd->data[RSP_DATA_IDX+2] == 0x55)

> +		*otp_data_is_locked = false;

> +	else

> +		*otp_data_is_locked = true;

> +

> +free_cmd:

> +	kfree(cmd);

> +	return ret;

> +}

> +

> +static int device_sanity_check(struct i2c_client *client)

> +{

> +	struct atmel_ecc_cmd *cmd;


when applying this patch you'll see that you will free this pointer without
allocating memory for it.

Best,
ta

> +	bool config_locked;

> +	bool otp_data_locked;

> +	int ret;

> +

>  	/*

>  	 * It is vital that the Configuration, Data and OTP zones be locked

>  	 * prior to release into the field of the system containing the device.

>  	 * Failure to lock these zones may permit modification of any secret

>  	 * keys and may lead to other security problems.

>  	 */

> -	if (cmd->data[RSP_DATA_IDX+3] == 0x55) {

> +	ret = atmel_ecc_get_lock_status(client, &config_locked,

> +					&otp_data_locked);

> +	if (ret)

> +		return ret;

> +

> +	if (!config_locked) {

>  		dev_err(&client->dev, "configuration zone is unlocked\n");

>  		ret = -ENOTSUPP;

>  	}

> -	if (cmd->data[RSP_DATA_IDX+2] == 0x55) {

> +	if (!otp_data_locked) {

>  		dev_err(&client->dev, "data and OTP zones are unlocked\n");

>  		ret = -ENOTSUPP;

>  	}

>

Patch

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 0dfea6eadc56..549ec7190287 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -666,7 +666,9 @@  static int atmel_ecc_get_serial(struct i2c_client *client)
 	return ret;
 }
 
-static int device_sanity_check(struct i2c_client *client)
+static int atmel_ecc_get_lock_status(struct i2c_client *client,
+				     bool *config_is_locked,
+				     bool *otp_data_is_locked)
 {
 	struct atmel_ecc_cmd *cmd;
 	int ret;
@@ -680,21 +682,49 @@  static int device_sanity_check(struct i2c_client *client)
 	ret = atmel_ecc_send_receive(client, cmd);
 	if (ret) {
 		dev_err(&client->dev,
-			"failed to send ECC init command\n");
+			"failed to send config read command\n");
 		goto free_cmd;
 	}
 
+	/* According to datasheet anything else than 0x55 means "locked" */
+	if (cmd->data[RSP_DATA_IDX+3] == 0x55)
+		*config_is_locked = false;
+	else
+		*config_is_locked = true;
+
+	if (cmd->data[RSP_DATA_IDX+2] == 0x55)
+		*otp_data_is_locked = false;
+	else
+		*otp_data_is_locked = true;
+
+free_cmd:
+	kfree(cmd);
+	return ret;
+}
+
+static int device_sanity_check(struct i2c_client *client)
+{
+	struct atmel_ecc_cmd *cmd;
+	bool config_locked;
+	bool otp_data_locked;
+	int ret;
+
 	/*
 	 * It is vital that the Configuration, Data and OTP zones be locked
 	 * prior to release into the field of the system containing the device.
 	 * Failure to lock these zones may permit modification of any secret
 	 * keys and may lead to other security problems.
 	 */
-	if (cmd->data[RSP_DATA_IDX+3] == 0x55) {
+	ret = atmel_ecc_get_lock_status(client, &config_locked,
+					&otp_data_locked);
+	if (ret)
+		return ret;
+
+	if (!config_locked) {
 		dev_err(&client->dev, "configuration zone is unlocked\n");
 		ret = -ENOTSUPP;
 	}
-	if (cmd->data[RSP_DATA_IDX+2] == 0x55) {
+	if (!otp_data_locked) {
 		dev_err(&client->dev, "data and OTP zones are unlocked\n");
 		ret = -ENOTSUPP;
 	}