[v3,1/7] sysfs/cpu: Allow individual architectures to select vulnerabilities

Message ID 20190109235544.2992426-2-jeremy.linton@arm.com
State New
Headers show
Series
  • arm64: add system vulnerability sysfs entries
Related show

Commit Message

Jeremy Linton Jan. 9, 2019, 11:55 p.m.
As suggested on the list, https://lkml.org/lkml/2019/1/4/282, there are
a number of cases where its useful for a system to avoid exporting a
sysfs entry for a given vulnerability. This set adds an architecture
specific callback which returns the bitmap of vulnerabilities the
architecture would like to advertise.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Kosina <jkosina@suse.cz>
---
 drivers/base/cpu.c  | 19 +++++++++++++++++++
 include/linux/cpu.h |  7 +++++++
 2 files changed, 26 insertions(+)

-- 
2.17.2

Comments

Suzuki K Poulose Jan. 14, 2019, 10:02 a.m. | #1
On 09/01/2019 23:55, Jeremy Linton wrote:
> As suggested on the list, https://lkml.org/lkml/2019/1/4/282, there are

> a number of cases where its useful for a system to avoid exporting a

> sysfs entry for a given vulnerability. This set adds an architecture

> specific callback which returns the bitmap of vulnerabilities the

> architecture would like to advertise.

> 

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> Cc: Rafael J. Wysocki <rafael@kernel.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>

> Cc: Josh Poimboeuf <jpoimboe@redhat.com>

> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> Cc: Ingo Molnar <mingo@kernel.org>

> Cc: Waiman Long <longman@redhat.com>

> Cc: Andi Kleen <ak@linux.intel.com>

> Cc: Jiri Kosina <jkosina@suse.cz>

> ---

>   drivers/base/cpu.c  | 19 +++++++++++++++++++

>   include/linux/cpu.h |  7 +++++++

>   2 files changed, 26 insertions(+)

> 

> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c

> index eb9443d5bae1..35f6dfb24cd6 100644

> --- a/drivers/base/cpu.c

> +++ b/drivers/base/cpu.c

> @@ -561,6 +561,11 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = {

>   	NULL

>   };

>   

> +uint __weak arch_supported_vuln_attr_fields(void)

> +{

> +	return VULN_MELTDOWN|VULN_SPECTREV1|VULN_SPECTREV2|VULN_SSB|VULN_L1TF;

> +}

> +

>   static const struct attribute_group cpu_root_vulnerabilities_group = {

>   	.name  = "vulnerabilities",

>   	.attrs = cpu_root_vulnerabilities_attrs,

> @@ -568,6 +573,20 @@ static const struct attribute_group cpu_root_vulnerabilities_group = {

>   

>   static void __init cpu_register_vulnerabilities(void)

>   {

> +	int fld;

> +	int max_fields = ARRAY_SIZE(cpu_root_vulnerabilities_attrs) - 1;

> +	struct attribute **hd = cpu_root_vulnerabilities_attrs;

> +	uint enabled_fields = arch_supported_vuln_attr_fields();

> +

> +	/* only enable entries requested by the arch code */

> +	for (fld = 0; fld < max_fields; fld++) {

> +		if (enabled_fields & 1 << fld) {

> +			*hd = cpu_root_vulnerabilities_attrs[fld];

> +			hd++;

> +		}

> +	}

> +	*hd = NULL;

> +


nit: Could we use "is_visible" callback in the attribute group to check this
dynamically ?

>   	if (sysfs_create_group(&cpu_subsys.dev_root->kobj,

>   			       &cpu_root_vulnerabilities_group))

>   		pr_err("Unable to register CPU vulnerabilities\n");

> diff --git a/include/linux/cpu.h b/include/linux/cpu.h

> index 218df7f4d3e1..5e45814bcc24 100644

> --- a/include/linux/cpu.h

> +++ b/include/linux/cpu.h

> @@ -189,4 +189,11 @@ static inline void cpu_smt_check_topology_early(void) { }

>   static inline void cpu_smt_check_topology(void) { }

>   #endif

>   

> +/* generic cpu vulnerability attributes */

> +#define VULN_MELTDOWN  0x01

> +#define VULN_SPECTREV1 0x02

> +#define VULN_SPECTREV2 0x04

> +#define VULN_SSB       0x08

> +#define VULN_L1TF      0x10


nit: May use BIT() ?

Cheers
Suzuki
Greg KH Jan. 18, 2019, 3:46 p.m. | #2
On Mon, Jan 14, 2019 at 10:02:21AM +0000, Suzuki K Poulose wrote:
> 

> 

> On 09/01/2019 23:55, Jeremy Linton wrote:

> > As suggested on the list, https://lkml.org/lkml/2019/1/4/282, there are

> > a number of cases where its useful for a system to avoid exporting a

> > sysfs entry for a given vulnerability. This set adds an architecture

> > specific callback which returns the bitmap of vulnerabilities the

> > architecture would like to advertise.

> > 

> > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> > Cc: Rafael J. Wysocki <rafael@kernel.org>

> > Cc: Thomas Gleixner <tglx@linutronix.de>

> > Cc: Josh Poimboeuf <jpoimboe@redhat.com>

> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> > Cc: Ingo Molnar <mingo@kernel.org>

> > Cc: Waiman Long <longman@redhat.com>

> > Cc: Andi Kleen <ak@linux.intel.com>

> > Cc: Jiri Kosina <jkosina@suse.cz>

> > ---

> >   drivers/base/cpu.c  | 19 +++++++++++++++++++

> >   include/linux/cpu.h |  7 +++++++

> >   2 files changed, 26 insertions(+)

> > 

> > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c

> > index eb9443d5bae1..35f6dfb24cd6 100644

> > --- a/drivers/base/cpu.c

> > +++ b/drivers/base/cpu.c

> > @@ -561,6 +561,11 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = {

> >   	NULL

> >   };

> > +uint __weak arch_supported_vuln_attr_fields(void)

> > +{

> > +	return VULN_MELTDOWN|VULN_SPECTREV1|VULN_SPECTREV2|VULN_SSB|VULN_L1TF;

> > +}

> > +

> >   static const struct attribute_group cpu_root_vulnerabilities_group = {

> >   	.name  = "vulnerabilities",

> >   	.attrs = cpu_root_vulnerabilities_attrs,

> > @@ -568,6 +573,20 @@ static const struct attribute_group cpu_root_vulnerabilities_group = {

> >   static void __init cpu_register_vulnerabilities(void)

> >   {

> > +	int fld;

> > +	int max_fields = ARRAY_SIZE(cpu_root_vulnerabilities_attrs) - 1;

> > +	struct attribute **hd = cpu_root_vulnerabilities_attrs;

> > +	uint enabled_fields = arch_supported_vuln_attr_fields();

> > +

> > +	/* only enable entries requested by the arch code */

> > +	for (fld = 0; fld < max_fields; fld++) {

> > +		if (enabled_fields & 1 << fld) {

> > +			*hd = cpu_root_vulnerabilities_attrs[fld];

> > +			hd++;

> > +		}

> > +	}

> > +	*hd = NULL;

> > +

> 

> nit: Could we use "is_visible" callback in the attribute group to check this

> dynamically ?


You should, that is what it is there for.

thanks,

greg k-h
Jeremy Linton Jan. 18, 2019, 4:31 p.m. | #3
On 01/18/2019 09:46 AM, Greg KH wrote:
> On Mon, Jan 14, 2019 at 10:02:21AM +0000, Suzuki K Poulose wrote:

>>

>>

>> On 09/01/2019 23:55, Jeremy Linton wrote:

>>> As suggested on the list, https://lkml.org/lkml/2019/1/4/282, there are

>>> a number of cases where its useful for a system to avoid exporting a

>>> sysfs entry for a given vulnerability. This set adds an architecture

>>> specific callback which returns the bitmap of vulnerabilities the

>>> architecture would like to advertise.

>>>

>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

>>> Cc: Rafael J. Wysocki <rafael@kernel.org>

>>> Cc: Thomas Gleixner <tglx@linutronix.de>

>>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>

>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

>>> Cc: Ingo Molnar <mingo@kernel.org>

>>> Cc: Waiman Long <longman@redhat.com>

>>> Cc: Andi Kleen <ak@linux.intel.com>

>>> Cc: Jiri Kosina <jkosina@suse.cz>

>>> ---

>>>    drivers/base/cpu.c  | 19 +++++++++++++++++++

>>>    include/linux/cpu.h |  7 +++++++

>>>    2 files changed, 26 insertions(+)

>>>

>>> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c

>>> index eb9443d5bae1..35f6dfb24cd6 100644

>>> --- a/drivers/base/cpu.c

>>> +++ b/drivers/base/cpu.c

>>> @@ -561,6 +561,11 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = {

>>>    	NULL

>>>    };

>>> +uint __weak arch_supported_vuln_attr_fields(void)

>>> +{

>>> +	return VULN_MELTDOWN|VULN_SPECTREV1|VULN_SPECTREV2|VULN_SSB|VULN_L1TF;

>>> +}

>>> +

>>>    static const struct attribute_group cpu_root_vulnerabilities_group = {

>>>    	.name  = "vulnerabilities",

>>>    	.attrs = cpu_root_vulnerabilities_attrs,

>>> @@ -568,6 +573,20 @@ static const struct attribute_group cpu_root_vulnerabilities_group = {

>>>    static void __init cpu_register_vulnerabilities(void)

>>>    {

>>> +	int fld;

>>> +	int max_fields = ARRAY_SIZE(cpu_root_vulnerabilities_attrs) - 1;

>>> +	struct attribute **hd = cpu_root_vulnerabilities_attrs;

>>> +	uint enabled_fields = arch_supported_vuln_attr_fields();

>>> +

>>> +	/* only enable entries requested by the arch code */

>>> +	for (fld = 0; fld < max_fields; fld++) {

>>> +		if (enabled_fields & 1 << fld) {

>>> +			*hd = cpu_root_vulnerabilities_attrs[fld];

>>> +			hd++;

>>> +		}

>>> +	}

>>> +	*hd = NULL;

>>> +

>>

>> nit: Could we use "is_visible" callback in the attribute group to check this

>> dynamically ?

> 

> You should, that is what it is there for.



Yes, its a good suggestion. OTOH, I think the plan is to drop this 
functionality all together by removing the ability to build kernels 
without the vulnerability checking/processor white lists. That will 
simplify some of the #ifdef'ing going on as well.

Patch

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index eb9443d5bae1..35f6dfb24cd6 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -561,6 +561,11 @@  static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	NULL
 };
 
+uint __weak arch_supported_vuln_attr_fields(void)
+{
+	return VULN_MELTDOWN|VULN_SPECTREV1|VULN_SPECTREV2|VULN_SSB|VULN_L1TF;
+}
+
 static const struct attribute_group cpu_root_vulnerabilities_group = {
 	.name  = "vulnerabilities",
 	.attrs = cpu_root_vulnerabilities_attrs,
@@ -568,6 +573,20 @@  static const struct attribute_group cpu_root_vulnerabilities_group = {
 
 static void __init cpu_register_vulnerabilities(void)
 {
+	int fld;
+	int max_fields = ARRAY_SIZE(cpu_root_vulnerabilities_attrs) - 1;
+	struct attribute **hd = cpu_root_vulnerabilities_attrs;
+	uint enabled_fields = arch_supported_vuln_attr_fields();
+
+	/* only enable entries requested by the arch code */
+	for (fld = 0; fld < max_fields; fld++) {
+		if (enabled_fields & 1 << fld) {
+			*hd = cpu_root_vulnerabilities_attrs[fld];
+			hd++;
+		}
+	}
+	*hd = NULL;
+
 	if (sysfs_create_group(&cpu_subsys.dev_root->kobj,
 			       &cpu_root_vulnerabilities_group))
 		pr_err("Unable to register CPU vulnerabilities\n");
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 218df7f4d3e1..5e45814bcc24 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -189,4 +189,11 @@  static inline void cpu_smt_check_topology_early(void) { }
 static inline void cpu_smt_check_topology(void) { }
 #endif
 
+/* generic cpu vulnerability attributes */
+#define VULN_MELTDOWN  0x01
+#define VULN_SPECTREV1 0x02
+#define VULN_SPECTREV2 0x04
+#define VULN_SSB       0x08
+#define VULN_L1TF      0x10
+
 #endif /* _LINUX_CPU_H_ */