[7/9,v2] crypto: atmel-ecc: Print out serial number

Message ID 20180628130729.17589-7-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 reads out the serial number of the crypto chip and prints it,
also toss this into the entropy pool as it is device-unique data.

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

---
ChangeLog v1->v2:
- kfree(cmd) was missed. Fix it with a goto construction.
- Coding style fixes.
---
 drivers/crypto/atmel-ecc.c | 60 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

-- 
2.17.0

Comments

Tudor Ambarus July 2, 2018, 3:10 p.m. | #1
Hi, Linus,

On 06/28/2018 04:07 PM, Linus Walleij wrote:
> This reads out the serial number of the crypto chip and prints it,

> also toss this into the entropy pool as it is device-unique data.

> 

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

> ---

> ChangeLog v1->v2:

> - kfree(cmd) was missed. Fix it with a goto construction.

> - Coding style fixes.

> ---

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

>  1 file changed, 60 insertions(+)

> 

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

> index f3322fae454e..659cb3cf37a9 100644

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

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

> @@ -26,6 +26,7 @@

>  #include <linux/kernel.h>

>  #include <linux/module.h>

>  #include <linux/of_device.h>

> +#include <linux/random.h>

>  #include <linux/scatterlist.h>

>  #include <linux/slab.h>

>  #include <linux/workqueue.h>

> @@ -610,6 +611,61 @@ static inline size_t atmel_ecc_wake_token_sz(u32 bus_clk_rate)

>  	return DIV_ROUND_UP(no_of_bits, 8);

>  }

>  

> +static int atmel_ecc_get_serial(struct i2c_client *client)

> +{

> +	struct atmel_ecc_cmd *cmd;

> +	int ret;

> +	int i;

> +	u8 serial[9];

> +

> +	cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);

> +	if (!cmd)

> +		return -ENOMEM;

> +

> +	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_0_3);

> +	ret = atmel_ecc_send_receive(client, cmd);

> +	if (ret) {

> +		dev_err(&client->dev,

> +			"failed to read serial byte 0-3\n");


the message can fit in the previous line.

> +		goto out_free_cmd;

> +	}

> +	for (i = 0; i < 4; i++)

> +		serial[i] = cmd->data[RSP_DATA_IDX + i];

> +

> +	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_4_7);

> +	ret = atmel_ecc_send_receive(client, cmd);

> +	if (ret) {

> +		dev_err(&client->dev,

> +			"failed to read serial byte 4-7\n");

> +		goto out_free_cmd;

> +	}

> +	for (i = 0; i < 4; i++)

> +		serial[4 + i] = cmd->data[RSP_DATA_IDX + i];

> +


unnecessary blank line. Some of these coding style issues can be catch with
checkpatch.pl --strict. There are few coding style checks in the next 2 patches too.

> +

> +	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN);

> +	ret = atmel_ecc_send_receive(client, cmd);

> +	if (ret) {

> +		kfree(cmd);


you will double free cmd, here and in out_free_cmd.

Otherwise looks good. The device supports 4-byte or 32-byte reads. You can also
do just one read of 32 bytes instead of three reads of four bytes each, but I
can't tell which one is better.

Thanks, Linus,
ta

> +		dev_err(&client->dev,

> +			"failed to read serial byte 8\n");

> +		goto out_free_cmd;

> +	}

> +	serial[8] = cmd->data[RSP_DATA_IDX];

> +

> +	/* This is device-unique data so it goes into the entropy pool */

> +	add_device_randomness(serial, sizeof(serial));

> +

> +	dev_info(&client->dev,

> +		 "serial number: %02x%02x%02x%02x%02x%02x%02x%02x%02x\n",

> +		 serial[0], serial[1], serial[2], serial[3], serial[4],

> +		 serial[5], serial[6], serial[7], serial[8]);

> +

> +out_free_cmd:

> +	kfree(cmd);

> +	return ret;

> +}

> +

>  static int device_sanity_check(struct i2c_client *client)

>  {

>  	struct atmel_ecc_cmd *cmd;

> @@ -692,6 +748,10 @@ static int atmel_ecc_probe(struct i2c_client *client,

>  	if (ret)

>  		return ret;

>  

> +	ret = atmel_ecc_get_serial(client);

> +	if (ret)

> +		return ret;

> +

>  	spin_lock(&driver_data.i2c_list_lock);

>  	list_add_tail(&i2c_priv->i2c_client_list_node,

>  		      &driver_data.i2c_client_list);

>

Patch

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index f3322fae454e..659cb3cf37a9 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -26,6 +26,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/random.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
@@ -610,6 +611,61 @@  static inline size_t atmel_ecc_wake_token_sz(u32 bus_clk_rate)
 	return DIV_ROUND_UP(no_of_bits, 8);
 }
 
+static int atmel_ecc_get_serial(struct i2c_client *client)
+{
+	struct atmel_ecc_cmd *cmd;
+	int ret;
+	int i;
+	u8 serial[9];
+
+	cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_0_3);
+	ret = atmel_ecc_send_receive(client, cmd);
+	if (ret) {
+		dev_err(&client->dev,
+			"failed to read serial byte 0-3\n");
+		goto out_free_cmd;
+	}
+	for (i = 0; i < 4; i++)
+		serial[i] = cmd->data[RSP_DATA_IDX + i];
+
+	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_4_7);
+	ret = atmel_ecc_send_receive(client, cmd);
+	if (ret) {
+		dev_err(&client->dev,
+			"failed to read serial byte 4-7\n");
+		goto out_free_cmd;
+	}
+	for (i = 0; i < 4; i++)
+		serial[4 + i] = cmd->data[RSP_DATA_IDX + i];
+
+
+	atmel_ecc_init_read_config_word(cmd, CONFIG_ZONE_SERIAL_8_I2CEN);
+	ret = atmel_ecc_send_receive(client, cmd);
+	if (ret) {
+		kfree(cmd);
+		dev_err(&client->dev,
+			"failed to read serial byte 8\n");
+		goto out_free_cmd;
+	}
+	serial[8] = cmd->data[RSP_DATA_IDX];
+
+	/* This is device-unique data so it goes into the entropy pool */
+	add_device_randomness(serial, sizeof(serial));
+
+	dev_info(&client->dev,
+		 "serial number: %02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
+		 serial[0], serial[1], serial[2], serial[3], serial[4],
+		 serial[5], serial[6], serial[7], serial[8]);
+
+out_free_cmd:
+	kfree(cmd);
+	return ret;
+}
+
 static int device_sanity_check(struct i2c_client *client)
 {
 	struct atmel_ecc_cmd *cmd;
@@ -692,6 +748,10 @@  static int atmel_ecc_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	ret = atmel_ecc_get_serial(client);
+	if (ret)
+		return ret;
+
 	spin_lock(&driver_data.i2c_list_lock);
 	list_add_tail(&i2c_priv->i2c_client_list_node,
 		      &driver_data.i2c_client_list);