Re: [PATCH 177/190] Revert "hwmon: (lm80) fix a missing check of bus read in lm80 probe"

[Guenter Roeck]

On 4/21/21 6:00 AM, Greg Kroah-Hartman wrote:
> This reverts commit 9aa3aa15f4c2f74f47afd6c5db4b420fadf3f315.
> 
> Commits from @umn.edu addresses have been found to be submitted in "bad
> faith" to try to test the kernel community's ability to review "known
> malicious" changes.  The result of these submissions can be found in a
> paper published at the 42nd IEEE Symposium on Security and Privacy
> entitled, "Open Source Insecurity: Stealthily Introducing
> Vulnerabilities via Hypocrite Commits" written by Qiushi Wu (University
> of Minnesota) and Kangjie Lu (University of Minnesota).
> 
> Because of this, all submissions from this group must be reverted from
> the kernel tree and will need to be re-reviewed again to determine if
> they actually are a valid fix.  Until that work is complete, remove this
> change to ensure that no problems are being introduced into the
> codebase.
> 
> Cc: Kangjie Lu <kjlu@xxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>

I don't immediately see a problem with this patch, but the other patch
introduced at the same time by the same author was indeed malicious,
and the bug it introduced was easy to overlook, so I agree that
this patch should be reverted.

[ When introducing error handling into this probe function, errors
  in lm80_init_client() should be addressed as well. This patch did
  not do that and thus had limited value anyway. ]

Thanks,
Guenter

> ---
>  drivers/hwmon/lm80.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/lm80.c b/drivers/hwmon/lm80.c
> index ac4adb44b224..97ab491d2922 100644
> --- a/drivers/hwmon/lm80.c
> +++ b/drivers/hwmon/lm80.c
> @@ -596,7 +596,6 @@ static int lm80_probe(struct i2c_client *client)
>  	struct device *dev = &client->dev;
>  	struct device *hwmon_dev;
>  	struct lm80_data *data;
> -	int rv;
>  
>  	data = devm_kzalloc(dev, sizeof(struct lm80_data), GFP_KERNEL);
>  	if (!data)
> @@ -609,14 +608,8 @@ static int lm80_probe(struct i2c_client *client)
>  	lm80_init_client(client);
>  
>  	/* A few vars need to be filled upon startup */
> -	rv = lm80_read_value(client, LM80_REG_FAN_MIN(1));
> -	if (rv < 0)
> -		return rv;
> -	data->fan[f_min][0] = rv;
> -	rv = lm80_read_value(client, LM80_REG_FAN_MIN(2));
> -	if (rv < 0)
> -		return rv;
> -	data->fan[f_min][1] = rv;
> +	data->fan[f_min][0] = lm80_read_value(client, LM80_REG_FAN_MIN(1));
> +	data->fan[f_min][1] = lm80_read_value(client, LM80_REG_FAN_MIN(2));
>  
>  	hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
>  							   data, lm80_groups);
>