linux-kernel - Re: [PATCH 1/2] iio: proximity: vl53l0x-i2c: Added sensor ID check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240831132437.1584a0e3@jic23-huawei>
Date: Sat, 31 Aug 2024 13:24:37 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Abhash Jha <abhashkumarjha123@...il.com>
Cc: linux-iio@...r.kernel.org, songqiang1304521@...il.com, lars@...afoo.de,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] iio: proximity: vl53l0x-i2c: Added sensor ID check

On Sat, 31 Aug 2024 01:46:25 +0530
Abhash Jha <abhashkumarjha123@...il.com> wrote:

> The commit adds a check for the sensor's model ID. We read the model
> identification register (0xC0) and expect a value of 0xEE.
> 
> Signed-off-by: Abhash Jha <abhashkumarjha123@...il.com>
> ---
>  drivers/iio/proximity/vl53l0x-i2c.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index 8d4f3f849..2b3dd18be 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -39,8 +39,11 @@
>  
>  #define VL_REG_RESULT_INT_STATUS			0x13
>  #define VL_REG_RESULT_RANGE_STATUS			0x14
> +#define VL_REG_IDENTIFICATION_MODEL_ID			0xC0
>  #define VL_REG_RESULT_RANGE_STATUS_COMPLETE		BIT(0)
>  
> +#define VL53L0X_MODEL_ID_VAL				0xEE
> +
>  struct vl53l0x_data {
>  	struct i2c_client *client;
>  	struct completion completion;
> @@ -223,6 +226,7 @@ static int vl53l0x_probe(struct i2c_client *client)
>  	struct vl53l0x_data *data;
>  	struct iio_dev *indio_dev;
>  	int error;
> +	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -237,6 +241,11 @@ static int vl53l0x_probe(struct i2c_client *client)
>  				     I2C_FUNC_SMBUS_BYTE_DATA))
>  		return -EOPNOTSUPP;
>  
> +	ret = i2c_smbus_read_byte_data(data->client, VL_REG_IDENTIFICATION_MODEL_ID);
> +	if (ret != VL53L0X_MODEL_ID_VAL)
> +		return dev_err_probe(&client->dev, ret,
This first ret should be the error return you want.  So -EINVAL or something like that.

> +				     "Received invalid model id: 0x%x", ret);

This needs to be message only, not an error return.

If we return an error here we break any future use of fallback device tree compatibles
for future devices running with an old kernel. 

So the most we should do is print a message that observes we don't recognise
the device, then carry on anyway.

We used to get this wrong in IIO and haven't yet fixed all drivers, but
I definitely don't want add such hard failures to more drivers.


> +
>  	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
>  	if (IS_ERR(data->vdd_supply))
>  		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
> @@ -265,8 +274,6 @@ static int vl53l0x_probe(struct i2c_client *client)
>  
>  	/* usage of interrupt is optional */
>  	if (client->irq) {
> -		int ret;
> -
>  		init_completion(&data->completion);
>  
>  		ret = vl53l0x_configure_irq(client, indio_dev);