diff mbox

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

Message ID 20180628130729.17589-7-linus.walleij@linaro.org (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show

Commit Message

Linus Walleij June 28, 2018, 1:07 p.m. UTC 
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(+)

Comments

Tudor Ambarus July 2, 2018, 3:10 p.m. UTC | #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);
>
diff mbox

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);