Message ID | 20220603152116.2269912-1-scott@os.amperecomputing.com (mailing list archive) |
---|---|
State | Rejected |
Headers | show |
Series | hwmon: (xgene) restrict power measurements to admin by default | expand |
On 6/3/22 08:21, D Scott Phillips wrote: > Access to power information can be used to infer the instructions being run > and possibly even data being processed on a cpu[1]. Restrict access to > power information to administrator users by default. (Cf. a similar > powercap change[2].) > > [1]: Lipp, Moritz, et al. "PLATYPUS: software-based power side-channel > attacks on x86." 2021 IEEE Symposium on Security and Privacy (SP). > IEEE, 2021. > [2]: commit 949dd0104c49 ("powercap: restrict energy meter to root access") > > Fixes: ed42cfa881e1 ("hwmon: Add xgene hwmon driver") > Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> > Cc: stable@vger.kernel.org > --- > drivers/hwmon/xgene-hwmon.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c > index 5cde837bfd09..6ad1daf2d354 100644 > --- a/drivers/hwmon/xgene-hwmon.c > +++ b/drivers/hwmon/xgene-hwmon.c > @@ -397,9 +397,9 @@ static DEVICE_ATTR_RO(temp1_label); > static DEVICE_ATTR_RO(temp1_input); > static DEVICE_ATTR_RO(temp1_critical_alarm); > static DEVICE_ATTR_RO(power1_label); > -static DEVICE_ATTR_RO(power1_input); > +static DEVICE_ATTR_ADMIN_RO(power1_input); > static DEVICE_ATTR_RO(power2_label); > -static DEVICE_ATTR_RO(power2_input); > +static DEVICE_ATTR_ADMIN_RO(power2_input); > > static struct attribute *xgene_hwmon_attrs[] = { > &dev_attr_temp1_label.attr, NACK. The hwmon ABI expects world read access. Either find a workaround by making the measurements less accurate, or drop the driver. Also see commit 9049572fb145 ("hwmon: Remove amd_energy driver"), "Attribute access" in Documentation/hwmon/sysfs-interface.rst, and [1]. Guenter --- [1] https://patchwork.kernel.org/project/linux-hwmon/patch/20210409174852.4585-2-linux@roeck-us.net/
Guenter Roeck <linux@roeck-us.net> writes: > On 6/3/22 08:21, D Scott Phillips wrote: >> Access to power information can be used to infer the instructions being run >> and possibly even data being processed on a cpu[1]. Restrict access to >> power information to administrator users by default. (Cf. a similar >> powercap change[2].) >> >> [1]: Lipp, Moritz, et al. "PLATYPUS: software-based power side-channel >> attacks on x86." 2021 IEEE Symposium on Security and Privacy (SP). >> IEEE, 2021. >> [2]: commit 949dd0104c49 ("powercap: restrict energy meter to root access") >> >> Fixes: ed42cfa881e1 ("hwmon: Add xgene hwmon driver") >> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/hwmon/xgene-hwmon.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c >> index 5cde837bfd09..6ad1daf2d354 100644 >> --- a/drivers/hwmon/xgene-hwmon.c >> +++ b/drivers/hwmon/xgene-hwmon.c >> @@ -397,9 +397,9 @@ static DEVICE_ATTR_RO(temp1_label); >> static DEVICE_ATTR_RO(temp1_input); >> static DEVICE_ATTR_RO(temp1_critical_alarm); >> static DEVICE_ATTR_RO(power1_label); >> -static DEVICE_ATTR_RO(power1_input); >> +static DEVICE_ATTR_ADMIN_RO(power1_input); >> static DEVICE_ATTR_RO(power2_label); >> -static DEVICE_ATTR_RO(power2_input); >> +static DEVICE_ATTR_ADMIN_RO(power2_input); >> >> static struct attribute *xgene_hwmon_attrs[] = { >> &dev_attr_temp1_label.attr, > > > NACK. The hwmon ABI expects world read access. Either find a workaround > by making the measurements less accurate, or drop the driver. Also see > commit 9049572fb145 ("hwmon: Remove amd_energy driver"), "Attribute access" > in Documentation/hwmon/sysfs-interface.rst, and [1]. Ah, I see, thanks for pointing that out, I had missed that. I'm not confident that the randomization approach can provide robustness against this attack, so I'll follow up with a patch to drop the driver. Cheers, Scott
diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c index 5cde837bfd09..6ad1daf2d354 100644 --- a/drivers/hwmon/xgene-hwmon.c +++ b/drivers/hwmon/xgene-hwmon.c @@ -397,9 +397,9 @@ static DEVICE_ATTR_RO(temp1_label); static DEVICE_ATTR_RO(temp1_input); static DEVICE_ATTR_RO(temp1_critical_alarm); static DEVICE_ATTR_RO(power1_label); -static DEVICE_ATTR_RO(power1_input); +static DEVICE_ATTR_ADMIN_RO(power1_input); static DEVICE_ATTR_RO(power2_label); -static DEVICE_ATTR_RO(power2_input); +static DEVICE_ATTR_ADMIN_RO(power2_input); static struct attribute *xgene_hwmon_attrs[] = { &dev_attr_temp1_label.attr,
Access to power information can be used to infer the instructions being run and possibly even data being processed on a cpu[1]. Restrict access to power information to administrator users by default. (Cf. a similar powercap change[2].) [1]: Lipp, Moritz, et al. "PLATYPUS: software-based power side-channel attacks on x86." 2021 IEEE Symposium on Security and Privacy (SP). IEEE, 2021. [2]: commit 949dd0104c49 ("powercap: restrict energy meter to root access") Fixes: ed42cfa881e1 ("hwmon: Add xgene hwmon driver") Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> Cc: stable@vger.kernel.org --- drivers/hwmon/xgene-hwmon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)