* [PATCH] hwmon: amd_energy: modify the visibility of the counters @ 2020-11-12 17:21 Naveen Krishna Chatradhi 2020-11-12 17:24 ` Guenter Roeck 0 siblings, 1 reply; 9+ messages in thread From: Naveen Krishna Chatradhi @ 2020-11-12 17:21 UTC (permalink / raw) To: linux-hwmon; +Cc: naveenkrishna.ch, Naveen Krishna Chatradhi, stable 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) -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-12 17:21 [PATCH] hwmon: amd_energy: modify the visibility of the counters Naveen Krishna Chatradhi @ 2020-11-12 17:24 ` Guenter Roeck 2020-11-13 13:58 ` Salvatore Bonaccorso 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2020-11-12 17:24 UTC (permalink / raw) To: Naveen Krishna Chatradhi, linux-hwmon; +Cc: naveenkrishna.ch, stable 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) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-12 17:24 ` Guenter Roeck @ 2020-11-13 13:58 ` Salvatore Bonaccorso 2020-11-13 14:47 ` Guenter Roeck 2020-11-22 6:56 ` Chatradhi, Naveen Krishna 0 siblings, 2 replies; 9+ messages in thread From: Salvatore Bonaccorso @ 2020-11-13 13:58 UTC (permalink / raw) To: Guenter Roeck Cc: Naveen Krishna Chatradhi, linux-hwmon, naveenkrishna.ch, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-13 13:58 ` Salvatore Bonaccorso @ 2020-11-13 14:47 ` Guenter Roeck 2020-11-22 6:56 ` Chatradhi, Naveen Krishna 1 sibling, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2020-11-13 14:47 UTC (permalink / raw) To: Salvatore Bonaccorso Cc: Naveen Krishna Chatradhi, linux-hwmon, naveenkrishna.ch, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-13 13:58 ` Salvatore Bonaccorso 2020-11-13 14:47 ` Guenter Roeck @ 2020-11-22 6:56 ` Chatradhi, Naveen Krishna 2020-11-22 13:30 ` Guenter Roeck 1 sibling, 1 reply; 9+ messages in thread From: Chatradhi, Naveen Krishna @ 2020-11-22 6:56 UTC (permalink / raw) To: Salvatore Bonaccorso, Guenter Roeck; +Cc: linux-hwmon, naveenkrishna.ch, stable [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.redhat.com%2Fshow_bug.cgi%3Fid%3D1897402&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=RCD5UPLJwh4NkUWf2Uq2r0PTYUC0f6DFDWLAQsrRJZI%3D&reserved=0 >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsupport.lenovo.com%2Flu%2Fuk%2Fproduct_security%2FLEN-50481&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=qBqjid0icKwjI%2Bz38twQqLUYwDzTfvCTF%2Bxzu0dXivY%3D&reserved=0 >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcve.mitre.org%2Fcgi-bin%2Fcvename.cgi%3Fname%3DCVE-2020-12912&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=xftV%2FNo3SvC3sHVKzq74m%2B4OmlYXKjSnSHjebcL%2FGQQ%3D&reserved=0 >> ? 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.redhat.com%2Fshow_bug.cgi%3Fid%3D1897402&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=RCD5UPLJwh4NkUWf2Uq2r0PTYUC0f6DFDWLAQsrRJZI%3D&reserved=0 https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsupport.lenovo.com%2Flu%2Fuk%2Fproduct_security%2FLEN-50481&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=qBqjid0icKwjI%2Bz38twQqLUYwDzTfvCTF%2Bxzu0dXivY%3D&reserved=0 https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcve.mitre.org%2Fcgi-bin%2Fcvename.cgi%3Fname%3DCVE-2020-12912&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C7672335ee2904d59fb5008d887dc381b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408727764403328%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=xftV%2FNo3SvC3sHVKzq74m%2B4OmlYXKjSnSHjebcL%2FGQQ%3D&reserved=0 ? Regards, Salvatore ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-22 6:56 ` Chatradhi, Naveen Krishna @ 2020-11-22 13:30 ` Guenter Roeck 2020-11-22 16:42 ` Chatradhi, Naveen Krishna 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2020-11-22 13:30 UTC (permalink / raw) To: Chatradhi, Naveen Krishna Cc: Salvatore Bonaccorso, linux-hwmon, naveenkrishna.ch, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-22 13:30 ` Guenter Roeck @ 2020-11-22 16:42 ` Chatradhi, Naveen Krishna 2020-11-22 18:09 ` Guenter Roeck 0 siblings, 1 reply; 9+ messages in thread From: Chatradhi, Naveen Krishna @ 2020-11-22 16:42 UTC (permalink / raw) To: Guenter Roeck; +Cc: Salvatore Bonaccorso, linux-hwmon, naveenkrishna.ch, stable [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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: amd_energy: modify the visibility of the counters 2020-11-22 16:42 ` Chatradhi, Naveen Krishna @ 2020-11-22 18:09 ` Guenter Roeck 0 siblings, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2020-11-22 18:09 UTC (permalink / raw) To: Chatradhi, Naveen Krishna Cc: Salvatore Bonaccorso, linux-hwmon, naveenkrishna.ch, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] hwmon: amd_energy: modify the visibility of the counters @ 2020-11-09 9:07 Naveen Krishna Chatradhi 0 siblings, 0 replies; 9+ messages in thread From: Naveen Krishna Chatradhi @ 2020-11-09 9:07 UTC (permalink / raw) To: security; +Cc: naveenkrishna.ch, Naveen Krishna Chatradhi, stable 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) -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-22 18:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-12 17:21 [PATCH] hwmon: amd_energy: modify the visibility of the counters Naveen Krishna Chatradhi 2020-11-12 17:24 ` Guenter Roeck 2020-11-13 13:58 ` Salvatore Bonaccorso 2020-11-13 14:47 ` Guenter Roeck 2020-11-22 6:56 ` Chatradhi, Naveen Krishna 2020-11-22 13:30 ` Guenter Roeck 2020-11-22 16:42 ` Chatradhi, Naveen Krishna 2020-11-22 18:09 ` Guenter Roeck -- strict thread matches above, loose matches on Subject: below -- 2020-11-09 9:07 Naveen Krishna Chatradhi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).