[PATCH v2 10/11] hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()

Andrew Davis posted 11 patches 1 month ago
[PATCH v2 10/11] hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()
Posted by Andrew Davis 1 month ago
The function i2c_match_id() is used to fetch the matching ID from
the i2c_device_id table. This can instead be done with
i2c_client_get_device_id(). For this driver functionality should
not change. Switch over to remove the last couple users of the
i2c_match_id() function from kernel.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/hwmon/pmbus/ltc2978.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
index 8f5be520a15db..d69a5e675e80e 100644
--- a/drivers/hwmon/pmbus/ltc2978.c
+++ b/drivers/hwmon/pmbus/ltc2978.c
@@ -733,7 +733,7 @@ static int ltc2978_probe(struct i2c_client *client)
 		return chip_id;
 
 	data->id = chip_id;
-	id = i2c_match_id(ltc2978_id, client);
+	id = i2c_client_get_device_id(client);
 	if (data->id != id->driver_data)
 		dev_warn(&client->dev,
 			 "Device mismatch: Configured %s (%d), detected %d\n",
-- 
2.39.2
Re: [PATCH v2 10/11] hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()
Posted by Guenter Roeck 1 month ago
On Fri, Mar 06, 2026 at 11:16:51AM -0600, Andrew Davis wrote:
> The function i2c_match_id() is used to fetch the matching ID from
> the i2c_device_id table. This can instead be done with
> i2c_client_get_device_id(). For this driver functionality should
> not change. Switch over to remove the last couple users of the
> i2c_match_id() function from kernel.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/hwmon/pmbus/ltc2978.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
> index 8f5be520a15db..d69a5e675e80e 100644
> --- a/drivers/hwmon/pmbus/ltc2978.c
> +++ b/drivers/hwmon/pmbus/ltc2978.c
> @@ -733,7 +733,7 @@ static int ltc2978_probe(struct i2c_client *client)
>  		return chip_id;
>  
>  	data->id = chip_id;
> -	id = i2c_match_id(ltc2978_id, client);
> +	id = i2c_client_get_device_id(client);

AI feedback:

  Is `id` guaranteed to be non-NULL here?

  If the device is instantiated via ACPI `PRP0001` or using a fallback DT
  compatible string where the first compatible string is not in the
  `ltc2978_id` table, `i2c_client_get_device_id()` will return `NULL`.
  This leads to a NULL pointer dereference when accessing `id->driver_data`.

  While this vulnerability existed in the old code with `i2c_match_id()`,
  adding a NULL check here might be a good idea while the code is being
  refactored.

I never know if this is real. Any idea ?

Thanks,
Guenter

>  	if (data->id != id->driver_data)
>  		dev_warn(&client->dev,
>  			 "Device mismatch: Configured %s (%d), detected %d\n",
> -- 
> 2.39.2
>
Re: [PATCH v2 10/11] hwmon: (pmbus/ltc2978) Remove use of i2c_match_id()
Posted by Andrew Davis 1 month ago
On 3/6/26 12:10 PM, Guenter Roeck wrote:
> On Fri, Mar 06, 2026 at 11:16:51AM -0600, Andrew Davis wrote:
>> The function i2c_match_id() is used to fetch the matching ID from
>> the i2c_device_id table. This can instead be done with
>> i2c_client_get_device_id(). For this driver functionality should
>> not change. Switch over to remove the last couple users of the
>> i2c_match_id() function from kernel.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/hwmon/pmbus/ltc2978.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
>> index 8f5be520a15db..d69a5e675e80e 100644
>> --- a/drivers/hwmon/pmbus/ltc2978.c
>> +++ b/drivers/hwmon/pmbus/ltc2978.c
>> @@ -733,7 +733,7 @@ static int ltc2978_probe(struct i2c_client *client)
>>   		return chip_id;
>>   
>>   	data->id = chip_id;
>> -	id = i2c_match_id(ltc2978_id, client);
>> +	id = i2c_client_get_device_id(client);
> 
> AI feedback:
> 
>    Is `id` guaranteed to be non-NULL here?
> 
>    If the device is instantiated via ACPI `PRP0001` or using a fallback DT
>    compatible string where the first compatible string is not in the
>    `ltc2978_id` table, `i2c_client_get_device_id()` will return `NULL`.
>    This leads to a NULL pointer dereference when accessing `id->driver_data`.
> 
>    While this vulnerability existed in the old code with `i2c_match_id()`,
>    adding a NULL check here might be a good idea while the code is being
>    refactored.
> 
> I never know if this is real. Any idea ?
> 

The AI is right on both parts, the second being the important one that
this was preexisting. i2c_match_id() should never return NULL in
practice as we must have matched on something to have gotten probe()'d
in the first place. And for the same reason i2c_client_get_device_id()
shouldn't ever return NULL either. So no change here.

But I see how this would confuse an AI as both functions have a
"return NULL;" statement in them. This problem of returning the matched
ID in-band with returning 0 for errors (which could also be a valid ID)
is one of the motivating reasons for me removing i2c_match_id(). Even
though its replacement has the same issue, once we get everyone over
to the same single API then switching that one API over to something
safer becomes possible.

Andrew

> Thanks,
> Guenter
> 
>>   	if (data->id != id->driver_data)
>>   		dev_warn(&client->dev,
>>   			 "Device mismatch: Configured %s (%d), detected %d\n",
>> -- 
>> 2.39.2
>>