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

Guenter Roeck
2021-04-21T14:03:50Z

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@umn.edu>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Guenter Roeck <linux@roeck-us.net>

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