This patch limits the visibility to owner and groups only for the energy counters exposed through the hwmon based amd_energy driver.
Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Naveen Krishna Chatradhi nchatrad@amd.com --- drivers/hwmon/amd_energy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c index d06597303d5a..3197cda7bcd9 100644 --- a/drivers/hwmon/amd_energy.c +++ b/drivers/hwmon/amd_energy.c @@ -171,7 +171,7 @@ static umode_t amd_energy_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr, int channel) { - return 0444; + return 0440; }
static int energy_accumulator(void *p)
On 11/12/20 9:21 AM, Naveen Krishna Chatradhi wrote:
This patch limits the visibility to owner and groups only for the energy counters exposed through the hwmon based amd_energy driver.
Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Naveen Krishna Chatradhi nchatrad@amd.com
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Guenter
drivers/hwmon/amd_energy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c index d06597303d5a..3197cda7bcd9 100644 --- a/drivers/hwmon/amd_energy.c +++ b/drivers/hwmon/amd_energy.c @@ -171,7 +171,7 @@ static umode_t amd_energy_is_visible(const void *_data, enum hwmon_sensor_types type, u32 attr, int channel) {
- return 0444;
- return 0440;
} static int energy_accumulator(void *p)
Hi,
On Thu, Nov 12, 2020 at 09:24:22AM -0800, Guenter Roeck wrote:
On 11/12/20 9:21 AM, Naveen Krishna Chatradhi wrote:
This patch limits the visibility to owner and groups only for the energy counters exposed through the hwmon based amd_energy driver.
Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Naveen Krishna Chatradhi nchatrad@amd.com
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Is this related to
https://bugzilla.redhat.com/show_bug.cgi?id=1897402 https://support.lenovo.com/lu/uk/product_security/LEN-50481 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-12912
?
Regards, Salvatore
On 11/13/20 5:58 AM, Salvatore Bonaccorso wrote:
Hi,
On Thu, Nov 12, 2020 at 09:24:22AM -0800, Guenter Roeck wrote:
On 11/12/20 9:21 AM, Naveen Krishna Chatradhi wrote:
This patch limits the visibility to owner and groups only for the energy counters exposed through the hwmon based amd_energy driver.
Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Naveen Krishna Chatradhi nchatrad@amd.com
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Is this related to
https://bugzilla.redhat.com/show_bug.cgi?id=1897402 https://support.lenovo.com/lu/uk/product_security/LEN-50481 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-12912
I guess so. The real fix would presumably be to read the power in the background. Of course, that won't work because reading it continuously or frequently causes power fluctuations. I'll apply the patch, but if there are complaints from users afterwards that "sensors" is broken I'll simply revert the entire driver.
Guenter
[AMD Official Use Only - Approved for External Use]
Hi Guenter, Salvatore
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Guenter, sorry for the delayed response. This fix is required to address the possible side channel attack reported in CVE-2020-12912.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsupport.le... https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcve.mitre....
?
Yes, Salvatore, thanks for bringing the links.
Regards, Naveenk
-----Original Message----- From: Salvatore Bonaccorso salvatore.bonaccorso@gmail.com On Behalf Of Salvatore Bonaccorso Sent: Friday, November 13, 2020 7:29 PM To: Guenter Roeck linux@roeck-us.net Cc: Chatradhi, Naveen Krishna NaveenKrishna.Chatradhi@amd.com; linux-hwmon@vger.kernel.org; naveenkrishna.ch@gmail.com; stable@vger.kernel.org Subject: Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters
[CAUTION: External Email]
Hi,
On Thu, Nov 12, 2020 at 09:24:22AM -0800, Guenter Roeck wrote:
On 11/12/20 9:21 AM, Naveen Krishna Chatradhi wrote:
This patch limits the visibility to owner and groups only for the energy counters exposed through the hwmon based amd_energy driver.
Cc: stable@vger.kernel.org Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Naveen Krishna Chatradhi nchatrad@amd.com
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Is this related to
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsupport.le... https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcve.mitre....
?
Regards, Salvatore
On Sun, Nov 22, 2020 at 06:56:24AM +0000, Chatradhi, Naveen Krishna wrote:
[AMD Official Use Only - Approved for External Use]
Hi Guenter, Salvatore
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Guenter, sorry for the delayed response. This fix is required to address the possible side channel attack reported in CVE-2020-12912.
[ ... ]
?
Yes, Salvatore, thanks for bringing the links.
A much better fix would have been to cache RAPL data for a short period of time. To avoid any possibility of attacks, maybe add some random interval. Something like this:
In accumulate_delta(): accums->next_update = jiffies + HZ / 2 + get_random_int % HZ;
In amd_energy_read(): accum = &data->accums[channel]; if (time_after(accum->next_update)) accumulate_delta(data, channel, cpu, reg); *val = div64_ul(accum->energy_ctr * 1000000UL, BIT(data->energy_units));
and drop amd_add_delta().
Guenter
[AMD Official Use Only - Approved for External Use]
Hi Guenter,
A much better fix would have been to cache RAPL data for a short period of time. To avoid any possibility of attacks, maybe add some random interval. Something like this:
Thanks for the tip, I will check this out.
In accumulate_delta(): accums->next_update = jiffies + HZ / 2 + get_random_int % HZ;
In amd_energy_read(): accum = &data->accums[channel]; if (time_after(accum->next_update))
Do you mean if (time_after(jiffies, accum->next_update))
accumulate_delta(data, channel, cpu, reg); *val = div64_ul(accum->energy_ctr * 1000000UL, BIT(data->energy_units));
and drop amd_add_delta().
Regards, Naveenk
-----Original Message----- From: Guenter Roeck linux@roeck-us.net Sent: Sunday, November 22, 2020 7:00 PM To: Chatradhi, Naveen Krishna NaveenKrishna.Chatradhi@amd.com Cc: Salvatore Bonaccorso carnil@debian.org; linux-hwmon@vger.kernel.org; naveenkrishna.ch@gmail.com; stable@vger.kernel.org Subject: Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters
[CAUTION: External Email]
On Sun, Nov 22, 2020 at 06:56:24AM +0000, Chatradhi, Naveen Krishna wrote:
[AMD Official Use Only - Approved for External Use]
Hi Guenter, Salvatore
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Guenter, sorry for the delayed response. This fix is required to address the possible side channel attack reported in CVE-2020-12912.
[ ... ]
?
Yes, Salvatore, thanks for bringing the links.
A much better fix would have been to cache RAPL data for a short period of time. To avoid any possibility of attacks, maybe add some random interval. Something like this:
In accumulate_delta(): accums->next_update = jiffies + HZ / 2 + get_random_int % HZ;
In amd_energy_read(): accum = &data->accums[channel]; if (time_after(accum->next_update)) accumulate_delta(data, channel, cpu, reg); *val = div64_ul(accum->energy_ctr * 1000000UL, BIT(data->energy_units));
and drop amd_add_delta().
Guenter
On Sun, Nov 22, 2020 at 04:42:47PM +0000, Chatradhi, Naveen Krishna wrote:
[AMD Official Use Only - Approved for External Use]
Hi Guenter,
A much better fix would have been to cache RAPL data for a short period of time. To avoid any possibility of attacks, maybe add some random interval. Something like this:
Thanks for the tip, I will check this out.
In accumulate_delta(): accums->next_update = jiffies + HZ / 2 + get_random_int % HZ;
[ and this was supposed to be get_random_int() ]
In amd_energy_read(): accum = &data->accums[channel]; if (time_after(accum->next_update))
Do you mean if (time_after(jiffies, accum->next_update))
yes ...
Guenter
accumulate_delta(data, channel, cpu, reg); *val = div64_ul(accum->energy_ctr * 1000000UL, BIT(data->energy_units));and drop amd_add_delta().
Regards, Naveenk
-----Original Message----- From: Guenter Roeck linux@roeck-us.net Sent: Sunday, November 22, 2020 7:00 PM To: Chatradhi, Naveen Krishna NaveenKrishna.Chatradhi@amd.com Cc: Salvatore Bonaccorso carnil@debian.org; linux-hwmon@vger.kernel.org; naveenkrishna.ch@gmail.com; stable@vger.kernel.org Subject: Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters
[CAUTION: External Email]
On Sun, Nov 22, 2020 at 06:56:24AM +0000, Chatradhi, Naveen Krishna wrote:
[AMD Official Use Only - Approved for External Use]
Hi Guenter, Salvatore
This is very unusual, and may mess up the "sensors" command. What problem is this trying to solve ?
Guenter, sorry for the delayed response. This fix is required to address the possible side channel attack reported in CVE-2020-12912.
[ ... ]
?
Yes, Salvatore, thanks for bringing the links.
A much better fix would have been to cache RAPL data for a short period of time. To avoid any possibility of attacks, maybe add some random interval. Something like this:
In accumulate_delta(): accums->next_update = jiffies + HZ / 2 + get_random_int % HZ;
In amd_energy_read(): accum = &data->accums[channel]; if (time_after(accum->next_update)) accumulate_delta(data, channel, cpu, reg); *val = div64_ul(accum->energy_ctr * 1000000UL, BIT(data->energy_units));
and drop amd_add_delta().
Guenter
linux-stable-mirror@lists.linaro.org
-
Chatradhi, Naveen Krishna -
Guenter Roeck -
Naveen Krishna Chatradhi -
Salvatore Bonaccorso