From: Dave Hansen <[email protected]>
== Problem ==
Nathan Chancellor reported an oops when aceessing the
'sgx_total_bytes' sysfs file:
https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
The sysfs output code accesses the sgx_numa_nodes[] array
unconditionally. However, this array is allocated during SGX
initialization, which only occurs on systems where SGX is
supported.
If the sysfs file is accessed on systems without SGX support,
sgx_numa_nodes[] is NULL and an oops occurs.
== Solution ==
To fix this, hide the entire nodeX/x86/ attribute group on
systems without SGX support using the ->is_visible attribute
group callback.
Unfortunately, SGX is initialized via a device_initcall() which
occurs _after_ the ->is_visible() callback. Instead of moving
SGX initialization earlier, call sysfs_update_group() during
SGX initialization to update the group visiblility.
This update requires moving the SGX sysfs code earlier in
sgx/main.c. There are no code changes other than the addition of
arch_update_sysfs_visibility() and a minor whitespace fixup to
arch_node_attr_is_visible() which checkpatch caught.
Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
b/arch/x86/kernel/cpu/sgx/main.c | 65 ++++++++++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 18 deletions(-)
diff -puN arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr arch/x86/kernel/cpu/sgx/main.c
--- a/arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr 2021-12-20 07:56:38.309584807 -0800
+++ b/arch/x86/kernel/cpu/sgx/main.c 2022-01-04 08:43:17.042821249 -0800
@@ -6,11 +6,13 @@
#include <linux/highmem.h>
#include <linux/kthread.h>
#include <linux/miscdevice.h>
+#include <linux/node.h>
#include <linux/pagemap.h>
#include <linux/ratelimit.h>
#include <linux/sched/mm.h>
#include <linux/sched/signal.h>
#include <linux/slab.h>
+#include <linux/sysfs.h>
#include <asm/sgx.h>
#include "driver.h"
#include "encl.h"
@@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_sectio
((high & GENMASK_ULL(19, 0)) << 32);
}
+#ifdef CONFIG_NUMA
+static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
+}
+static DEVICE_ATTR_RO(sgx_total_bytes);
+
+static umode_t arch_node_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ /* Make all x86/ attributes invisible when SGX is not initialized: */
+ if (nodes_empty(sgx_numa_mask))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute *arch_node_dev_attrs[] = {
+ &dev_attr_sgx_total_bytes.attr,
+ NULL,
+};
+
+const struct attribute_group arch_node_dev_group = {
+ .name = "x86",
+ .attrs = arch_node_dev_attrs,
+ .is_visible = arch_node_attr_is_visible,
+};
+
+static void __init arch_update_sysfs_visibility(int nid)
+{
+ struct node *node = node_devices[nid];
+ int ret;
+
+ ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
+
+ if (ret)
+ pr_err("sysfs update failed (%d), files may be invisible", ret);
+}
+#else /* !CONFIG_NUMA */
+static void __init arch_update_sysfs_visibility(int nid) {}
+#endif
+
static bool __init sgx_page_cache_init(void)
{
u32 eax, ebx, ecx, edx, type;
@@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(v
INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
node_set(nid, sgx_numa_mask);
sgx_numa_nodes[nid].size = 0;
+
+ /* Make SGX-specific node sysfs files visible: */
+ arch_update_sysfs_visibility(nid);
}
sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
@@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *all
}
EXPORT_SYMBOL_GPL(sgx_set_attribute);
-#ifdef CONFIG_NUMA
-static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
- return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
-}
-static DEVICE_ATTR_RO(sgx_total_bytes);
-
-static struct attribute *arch_node_dev_attrs[] = {
- &dev_attr_sgx_total_bytes.attr,
- NULL,
-};
-
-const struct attribute_group arch_node_dev_group = {
- .name = "x86",
- .attrs = arch_node_dev_attrs,
-};
-#endif /* CONFIG_NUMA */
-
static int __init sgx_init(void)
{
int ret;
_
On Tue, Jan 04, 2022 at 09:15:27AM -0800, Dave Hansen wrote:
>
> From: Dave Hansen <[email protected]>
>
> == Problem ==
>
> Nathan Chancellor reported an oops when aceessing the
> 'sgx_total_bytes' sysfs file:
>
> https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
>
> The sysfs output code accesses the sgx_numa_nodes[] array
> unconditionally. However, this array is allocated during SGX
> initialization, which only occurs on systems where SGX is
> supported.
>
> If the sysfs file is accessed on systems without SGX support,
> sgx_numa_nodes[] is NULL and an oops occurs.
>
> == Solution ==
>
> To fix this, hide the entire nodeX/x86/ attribute group on
> systems without SGX support using the ->is_visible attribute
> group callback.
>
> Unfortunately, SGX is initialized via a device_initcall() which
> occurs _after_ the ->is_visible() callback. Instead of moving
> SGX initialization earlier, call sysfs_update_group() during
> SGX initialization to update the group visiblility.
>
> This update requires moving the SGX sysfs code earlier in
> sgx/main.c. There are no code changes other than the addition of
> arch_update_sysfs_visibility() and a minor whitespace fixup to
> arch_node_attr_is_visible() which checkpatch caught.
>
> Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> CC: Greg Kroah-Hartman <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
Tested-by: Nathan Chancellor <[email protected]>
> ---
>
> b/arch/x86/kernel/cpu/sgx/main.c | 65 ++++++++++++++++++++++++++++-----------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff -puN arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr arch/x86/kernel/cpu/sgx/main.c
> --- a/arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr 2021-12-20 07:56:38.309584807 -0800
> +++ b/arch/x86/kernel/cpu/sgx/main.c 2022-01-04 08:43:17.042821249 -0800
> @@ -6,11 +6,13 @@
> #include <linux/highmem.h>
> #include <linux/kthread.h>
> #include <linux/miscdevice.h>
> +#include <linux/node.h>
> #include <linux/pagemap.h>
> #include <linux/ratelimit.h>
> #include <linux/sched/mm.h>
> #include <linux/sched/signal.h>
> #include <linux/slab.h>
> +#include <linux/sysfs.h>
> #include <asm/sgx.h>
> #include "driver.h"
> #include "encl.h"
> @@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_sectio
> ((high & GENMASK_ULL(19, 0)) << 32);
> }
>
> +#ifdef CONFIG_NUMA
> +static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> +}
> +static DEVICE_ATTR_RO(sgx_total_bytes);
> +
> +static umode_t arch_node_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + /* Make all x86/ attributes invisible when SGX is not initialized: */
> + if (nodes_empty(sgx_numa_mask))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute *arch_node_dev_attrs[] = {
> + &dev_attr_sgx_total_bytes.attr,
> + NULL,
> +};
> +
> +const struct attribute_group arch_node_dev_group = {
> + .name = "x86",
> + .attrs = arch_node_dev_attrs,
> + .is_visible = arch_node_attr_is_visible,
> +};
> +
> +static void __init arch_update_sysfs_visibility(int nid)
> +{
> + struct node *node = node_devices[nid];
> + int ret;
> +
> + ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
> +
> + if (ret)
> + pr_err("sysfs update failed (%d), files may be invisible", ret);
> +}
> +#else /* !CONFIG_NUMA */
> +static void __init arch_update_sysfs_visibility(int nid) {}
> +#endif
> +
> static bool __init sgx_page_cache_init(void)
> {
> u32 eax, ebx, ecx, edx, type;
> @@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(v
> INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
> node_set(nid, sgx_numa_mask);
> sgx_numa_nodes[nid].size = 0;
> +
> + /* Make SGX-specific node sysfs files visible: */
> + arch_update_sysfs_visibility(nid);
> }
>
> sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
> @@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *all
> }
> EXPORT_SYMBOL_GPL(sgx_set_attribute);
>
> -#ifdef CONFIG_NUMA
> -static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> - return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> -}
> -static DEVICE_ATTR_RO(sgx_total_bytes);
> -
> -static struct attribute *arch_node_dev_attrs[] = {
> - &dev_attr_sgx_total_bytes.attr,
> - NULL,
> -};
> -
> -const struct attribute_group arch_node_dev_group = {
> - .name = "x86",
> - .attrs = arch_node_dev_attrs,
> -};
> -#endif /* CONFIG_NUMA */
> -
> static int __init sgx_init(void)
> {
> int ret;
> _
On Tue, Jan 04, 2022 at 09:15:27AM -0800, Dave Hansen wrote:
>
> From: Dave Hansen <[email protected]>
>
> == Problem ==
>
> Nathan Chancellor reported an oops when aceessing the
> 'sgx_total_bytes' sysfs file:
>
> https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
>
> The sysfs output code accesses the sgx_numa_nodes[] array
> unconditionally. However, this array is allocated during SGX
> initialization, which only occurs on systems where SGX is
> supported.
>
> If the sysfs file is accessed on systems without SGX support,
> sgx_numa_nodes[] is NULL and an oops occurs.
>
> == Solution ==
>
> To fix this, hide the entire nodeX/x86/ attribute group on
> systems without SGX support using the ->is_visible attribute
> group callback.
>
> Unfortunately, SGX is initialized via a device_initcall() which
> occurs _after_ the ->is_visible() callback. Instead of moving
> SGX initialization earlier, call sysfs_update_group() during
> SGX initialization to update the group visiblility.
>
> This update requires moving the SGX sysfs code earlier in
> sgx/main.c. There are no code changes other than the addition of
> arch_update_sysfs_visibility() and a minor whitespace fixup to
> arch_node_attr_is_visible() which checkpatch caught.
>
> Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> CC: Greg Kroah-Hartman <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
>
> b/arch/x86/kernel/cpu/sgx/main.c | 65 ++++++++++++++++++++++++++++-----------
> 1 file changed, 47 insertions(+), 18 deletions(-)
Reviewed-by: Greg Kroah-Hartman <[email protected]>
On Tue, Jan 04, 2022 at 09:15:27AM -0800, Dave Hansen wrote:
>
> From: Dave Hansen <[email protected]>
>
> == Problem ==
>
> Nathan Chancellor reported an oops when aceessing the
> 'sgx_total_bytes' sysfs file:
>
> https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
>
> The sysfs output code accesses the sgx_numa_nodes[] array
> unconditionally. However, this array is allocated during SGX
> initialization, which only occurs on systems where SGX is
> supported.
>
> If the sysfs file is accessed on systems without SGX support,
> sgx_numa_nodes[] is NULL and an oops occurs.
>
> == Solution ==
>
> To fix this, hide the entire nodeX/x86/ attribute group on
> systems without SGX support using the ->is_visible attribute
> group callback.
>
> Unfortunately, SGX is initialized via a device_initcall() which
> occurs _after_ the ->is_visible() callback. Instead of moving
> SGX initialization earlier, call sysfs_update_group() during
> SGX initialization to update the group visiblility.
>
> This update requires moving the SGX sysfs code earlier in
> sgx/main.c. There are no code changes other than the addition of
> arch_update_sysfs_visibility() and a minor whitespace fixup to
> arch_node_attr_is_visible() which checkpatch caught.
>
> Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> CC: Greg Kroah-Hartman <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
>
> b/arch/x86/kernel/cpu/sgx/main.c | 65 ++++++++++++++++++++++++++++-----------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff -puN arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr arch/x86/kernel/cpu/sgx/main.c
> --- a/arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr 2021-12-20 07:56:38.309584807 -0800
> +++ b/arch/x86/kernel/cpu/sgx/main.c 2022-01-04 08:43:17.042821249 -0800
> @@ -6,11 +6,13 @@
> #include <linux/highmem.h>
> #include <linux/kthread.h>
> #include <linux/miscdevice.h>
> +#include <linux/node.h>
> #include <linux/pagemap.h>
> #include <linux/ratelimit.h>
> #include <linux/sched/mm.h>
> #include <linux/sched/signal.h>
> #include <linux/slab.h>
> +#include <linux/sysfs.h>
> #include <asm/sgx.h>
> #include "driver.h"
> #include "encl.h"
> @@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_sectio
> ((high & GENMASK_ULL(19, 0)) << 32);
> }
>
> +#ifdef CONFIG_NUMA
> +static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> +}
> +static DEVICE_ATTR_RO(sgx_total_bytes);
> +
> +static umode_t arch_node_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + /* Make all x86/ attributes invisible when SGX is not initialized: */
> + if (nodes_empty(sgx_numa_mask))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute *arch_node_dev_attrs[] = {
> + &dev_attr_sgx_total_bytes.attr,
> + NULL,
> +};
> +
> +const struct attribute_group arch_node_dev_group = {
> + .name = "x86",
> + .attrs = arch_node_dev_attrs,
> + .is_visible = arch_node_attr_is_visible,
> +};
> +
> +static void __init arch_update_sysfs_visibility(int nid)
> +{
> + struct node *node = node_devices[nid];
> + int ret;
> +
> + ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
> +
> + if (ret)
> + pr_err("sysfs update failed (%d), files may be invisible", ret);
> +}
> +#else /* !CONFIG_NUMA */
> +static void __init arch_update_sysfs_visibility(int nid) {}
> +#endif
> +
> static bool __init sgx_page_cache_init(void)
> {
> u32 eax, ebx, ecx, edx, type;
> @@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(v
> INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
> node_set(nid, sgx_numa_mask);
> sgx_numa_nodes[nid].size = 0;
> +
> + /* Make SGX-specific node sysfs files visible: */
> + arch_update_sysfs_visibility(nid);
> }
>
> sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
> @@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *all
> }
> EXPORT_SYMBOL_GPL(sgx_set_attribute);
>
> -#ifdef CONFIG_NUMA
> -static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> - return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> -}
> -static DEVICE_ATTR_RO(sgx_total_bytes);
> -
> -static struct attribute *arch_node_dev_attrs[] = {
> - &dev_attr_sgx_total_bytes.attr,
> - NULL,
> -};
> -
> -const struct attribute_group arch_node_dev_group = {
> - .name = "x86",
> - .attrs = arch_node_dev_attrs,
> -};
> -#endif /* CONFIG_NUMA */
> -
> static int __init sgx_init(void)
> {
> int ret;
> _
Please add both:
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>
I explained the testing procedure in the other email.
/Jarkko
On Fri, Jan 07, 2022 at 01:47:00PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jan 04, 2022 at 09:15:27AM -0800, Dave Hansen wrote:
> >
> > From: Dave Hansen <[email protected]>
> >
> > == Problem ==
> >
> > Nathan Chancellor reported an oops when aceessing the
> > 'sgx_total_bytes' sysfs file:
> >
> > https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
> >
> > The sysfs output code accesses the sgx_numa_nodes[] array
> > unconditionally. However, this array is allocated during SGX
> > initialization, which only occurs on systems where SGX is
> > supported.
> >
> > If the sysfs file is accessed on systems without SGX support,
> > sgx_numa_nodes[] is NULL and an oops occurs.
> >
> > == Solution ==
> >
> > To fix this, hide the entire nodeX/x86/ attribute group on
> > systems without SGX support using the ->is_visible attribute
> > group callback.
> >
> > Unfortunately, SGX is initialized via a device_initcall() which
> > occurs _after_ the ->is_visible() callback. Instead of moving
> > SGX initialization earlier, call sysfs_update_group() during
> > SGX initialization to update the group visiblility.
> >
> > This update requires moving the SGX sysfs code earlier in
> > sgx/main.c. There are no code changes other than the addition of
> > arch_update_sysfs_visibility() and a minor whitespace fixup to
> > arch_node_attr_is_visible() which checkpatch caught.
> >
> > Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
> > Reported-by: Nathan Chancellor <[email protected]>
> > Signed-off-by: Dave Hansen <[email protected]>
> > CC: Greg Kroah-Hartman <[email protected]>
> > Cc: Jarkko Sakkinen <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> >
> > b/arch/x86/kernel/cpu/sgx/main.c | 65 ++++++++++++++++++++++++++++-----------
> > 1 file changed, 47 insertions(+), 18 deletions(-)
> >
> > diff -puN arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr arch/x86/kernel/cpu/sgx/main.c
> > --- a/arch/x86/kernel/cpu/sgx/main.c~sgx-null-ptr 2021-12-20 07:56:38.309584807 -0800
> > +++ b/arch/x86/kernel/cpu/sgx/main.c 2022-01-04 08:43:17.042821249 -0800
> > @@ -6,11 +6,13 @@
> > #include <linux/highmem.h>
> > #include <linux/kthread.h>
> > #include <linux/miscdevice.h>
> > +#include <linux/node.h>
> > #include <linux/pagemap.h>
> > #include <linux/ratelimit.h>
> > #include <linux/sched/mm.h>
> > #include <linux/sched/signal.h>
> > #include <linux/slab.h>
> > +#include <linux/sysfs.h>
> > #include <asm/sgx.h>
> > #include "driver.h"
> > #include "encl.h"
> > @@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_sectio
> > ((high & GENMASK_ULL(19, 0)) << 32);
> > }
> >
> > +#ifdef CONFIG_NUMA
> > +static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> > +}
> > +static DEVICE_ATTR_RO(sgx_total_bytes);
> > +
> > +static umode_t arch_node_attr_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int idx)
> > +{
> > + /* Make all x86/ attributes invisible when SGX is not initialized: */
> > + if (nodes_empty(sgx_numa_mask))
> > + return 0;
> > +
> > + return attr->mode;
> > +}
> > +
> > +static struct attribute *arch_node_dev_attrs[] = {
> > + &dev_attr_sgx_total_bytes.attr,
> > + NULL,
> > +};
> > +
> > +const struct attribute_group arch_node_dev_group = {
> > + .name = "x86",
> > + .attrs = arch_node_dev_attrs,
> > + .is_visible = arch_node_attr_is_visible,
> > +};
> > +
> > +static void __init arch_update_sysfs_visibility(int nid)
> > +{
> > + struct node *node = node_devices[nid];
> > + int ret;
> > +
> > + ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
> > +
> > + if (ret)
> > + pr_err("sysfs update failed (%d), files may be invisible", ret);
> > +}
> > +#else /* !CONFIG_NUMA */
> > +static void __init arch_update_sysfs_visibility(int nid) {}
> > +#endif
> > +
> > static bool __init sgx_page_cache_init(void)
> > {
> > u32 eax, ebx, ecx, edx, type;
> > @@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(v
> > INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
> > node_set(nid, sgx_numa_mask);
> > sgx_numa_nodes[nid].size = 0;
> > +
> > + /* Make SGX-specific node sysfs files visible: */
> > + arch_update_sysfs_visibility(nid);
> > }
> >
> > sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
> > @@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *all
> > }
> > EXPORT_SYMBOL_GPL(sgx_set_attribute);
> >
> > -#ifdef CONFIG_NUMA
> > -static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
> > -{
> > - return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
> > -}
> > -static DEVICE_ATTR_RO(sgx_total_bytes);
> > -
> > -static struct attribute *arch_node_dev_attrs[] = {
> > - &dev_attr_sgx_total_bytes.attr,
> > - NULL,
> > -};
> > -
> > -const struct attribute_group arch_node_dev_group = {
> > - .name = "x86",
> > - .attrs = arch_node_dev_attrs,
> > -};
> > -#endif /* CONFIG_NUMA */
> > -
> > static int __init sgx_init(void)
> > {
> > int ret;
> > _
>
> Please add both:
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> I explained the testing procedure in the other email.
To add: I tested both this final version and also the earlier
attached version.
/Jarkko
The following commit has been merged into the x86/sgx branch of tip:
Commit-ID: cebb880030045059e55d21cbe049cdfa18d3990d
Gitweb: https://git.kernel.org/tip/cebb880030045059e55d21cbe049cdfa18d3990d
Author: Dave Hansen <[email protected]>
AuthorDate: Tue, 04 Jan 2022 09:15:27 -08:00
Committer: Dave Hansen <[email protected]>
CommitterDate: Fri, 07 Jan 2022 08:42:33 -08:00
x86/sgx: Fix NULL pointer dereference on non-SGX systems
== Problem ==
Nathan Chancellor reported an oops when aceessing the
'sgx_total_bytes' sysfs file:
https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
The sysfs output code accesses the sgx_numa_nodes[] array
unconditionally. However, this array is allocated during SGX
initialization, which only occurs on systems where SGX is
supported.
If the sysfs file is accessed on systems without SGX support,
sgx_numa_nodes[] is NULL and an oops occurs.
== Solution ==
To fix this, hide the entire nodeX/x86/ attribute group on
systems without SGX support using the ->is_visible attribute
group callback.
Unfortunately, SGX is initialized via a device_initcall() which
occurs _after_ the ->is_visible() callback. Instead of moving
SGX initialization earlier, call sysfs_update_group() during
SGX initialization to update the group visiblility.
This update requires moving the SGX sysfs code earlier in
sgx/main.c. There are no code changes other than the addition of
arch_update_sysfs_visibility() and a minor whitespace fixup to
arch_node_attr_is_visible() which checkpatch caught.
CC: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/cpu/sgx/main.c | 65 +++++++++++++++++++++++----------
1 file changed, 47 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 2857a49..4b41efc 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -6,11 +6,13 @@
#include <linux/highmem.h>
#include <linux/kthread.h>
#include <linux/miscdevice.h>
+#include <linux/node.h>
#include <linux/pagemap.h>
#include <linux/ratelimit.h>
#include <linux/sched/mm.h>
#include <linux/sched/signal.h>
#include <linux/slab.h>
+#include <linux/sysfs.h>
#include <asm/sgx.h>
#include "driver.h"
#include "encl.h"
@@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_section_metric(u64 low, u64 high)
((high & GENMASK_ULL(19, 0)) << 32);
}
+#ifdef CONFIG_NUMA
+static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
+}
+static DEVICE_ATTR_RO(sgx_total_bytes);
+
+static umode_t arch_node_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ /* Make all x86/ attributes invisible when SGX is not initialized: */
+ if (nodes_empty(sgx_numa_mask))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute *arch_node_dev_attrs[] = {
+ &dev_attr_sgx_total_bytes.attr,
+ NULL,
+};
+
+const struct attribute_group arch_node_dev_group = {
+ .name = "x86",
+ .attrs = arch_node_dev_attrs,
+ .is_visible = arch_node_attr_is_visible,
+};
+
+static void __init arch_update_sysfs_visibility(int nid)
+{
+ struct node *node = node_devices[nid];
+ int ret;
+
+ ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
+
+ if (ret)
+ pr_err("sysfs update failed (%d), files may be invisible", ret);
+}
+#else /* !CONFIG_NUMA */
+static void __init arch_update_sysfs_visibility(int nid) {}
+#endif
+
static bool __init sgx_page_cache_init(void)
{
u32 eax, ebx, ecx, edx, type;
@@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(void)
INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
node_set(nid, sgx_numa_mask);
sgx_numa_nodes[nid].size = 0;
+
+ /* Make SGX-specific node sysfs files visible: */
+ arch_update_sysfs_visibility(nid);
}
sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
@@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
}
EXPORT_SYMBOL_GPL(sgx_set_attribute);
-#ifdef CONFIG_NUMA
-static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
- return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
-}
-static DEVICE_ATTR_RO(sgx_total_bytes);
-
-static struct attribute *arch_node_dev_attrs[] = {
- &dev_attr_sgx_total_bytes.attr,
- NULL,
-};
-
-const struct attribute_group arch_node_dev_group = {
- .name = "x86",
- .attrs = arch_node_dev_attrs,
-};
-#endif /* CONFIG_NUMA */
-
static int __init sgx_init(void)
{
int ret;
The following commit has been merged into the x86/sgx branch of tip:
Commit-ID: 2056e2989bf47ad7274ecc5e9dda2add53c112f9
Gitweb: https://git.kernel.org/tip/2056e2989bf47ad7274ecc5e9dda2add53c112f9
Author: Dave Hansen <[email protected]>
AuthorDate: Tue, 04 Jan 2022 09:15:27 -08:00
Committer: Dave Hansen <[email protected]>
CommitterDate: Fri, 07 Jan 2022 08:47:23 -08:00
x86/sgx: Fix NULL pointer dereference on non-SGX systems
== Problem ==
Nathan Chancellor reported an oops when aceessing the
'sgx_total_bytes' sysfs file:
https://lore.kernel.org/all/YbzhBrimHGGpddDM@archlinux-ax161/
The sysfs output code accesses the sgx_numa_nodes[] array
unconditionally. However, this array is allocated during SGX
initialization, which only occurs on systems where SGX is
supported.
If the sysfs file is accessed on systems without SGX support,
sgx_numa_nodes[] is NULL and an oops occurs.
== Solution ==
To fix this, hide the entire nodeX/x86/ attribute group on
systems without SGX support using the ->is_visible attribute
group callback.
Unfortunately, SGX is initialized via a device_initcall() which
occurs _after_ the ->is_visible() callback. Instead of moving
SGX initialization earlier, call sysfs_update_group() during
SGX initialization to update the group visiblility.
This update requires moving the SGX sysfs code earlier in
sgx/main.c. There are no code changes other than the addition of
arch_update_sysfs_visibility() and a minor whitespace fixup to
arch_node_attr_is_visible() which checkpatch caught.
CC: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Fixes: 50468e431335 ("x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node")
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/cpu/sgx/main.c | 65 +++++++++++++++++++++++----------
1 file changed, 47 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 2857a49..4b41efc 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -6,11 +6,13 @@
#include <linux/highmem.h>
#include <linux/kthread.h>
#include <linux/miscdevice.h>
+#include <linux/node.h>
#include <linux/pagemap.h>
#include <linux/ratelimit.h>
#include <linux/sched/mm.h>
#include <linux/sched/signal.h>
#include <linux/slab.h>
+#include <linux/sysfs.h>
#include <asm/sgx.h>
#include "driver.h"
#include "encl.h"
@@ -780,6 +782,48 @@ static inline u64 __init sgx_calc_section_metric(u64 low, u64 high)
((high & GENMASK_ULL(19, 0)) << 32);
}
+#ifdef CONFIG_NUMA
+static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
+}
+static DEVICE_ATTR_RO(sgx_total_bytes);
+
+static umode_t arch_node_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ /* Make all x86/ attributes invisible when SGX is not initialized: */
+ if (nodes_empty(sgx_numa_mask))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute *arch_node_dev_attrs[] = {
+ &dev_attr_sgx_total_bytes.attr,
+ NULL,
+};
+
+const struct attribute_group arch_node_dev_group = {
+ .name = "x86",
+ .attrs = arch_node_dev_attrs,
+ .is_visible = arch_node_attr_is_visible,
+};
+
+static void __init arch_update_sysfs_visibility(int nid)
+{
+ struct node *node = node_devices[nid];
+ int ret;
+
+ ret = sysfs_update_group(&node->dev.kobj, &arch_node_dev_group);
+
+ if (ret)
+ pr_err("sysfs update failed (%d), files may be invisible", ret);
+}
+#else /* !CONFIG_NUMA */
+static void __init arch_update_sysfs_visibility(int nid) {}
+#endif
+
static bool __init sgx_page_cache_init(void)
{
u32 eax, ebx, ecx, edx, type;
@@ -826,6 +870,9 @@ static bool __init sgx_page_cache_init(void)
INIT_LIST_HEAD(&sgx_numa_nodes[nid].sgx_poison_page_list);
node_set(nid, sgx_numa_mask);
sgx_numa_nodes[nid].size = 0;
+
+ /* Make SGX-specific node sysfs files visible: */
+ arch_update_sysfs_visibility(nid);
}
sgx_epc_sections[i].node = &sgx_numa_nodes[nid];
@@ -903,24 +950,6 @@ int sgx_set_attribute(unsigned long *allowed_attributes,
}
EXPORT_SYMBOL_GPL(sgx_set_attribute);
-#ifdef CONFIG_NUMA
-static ssize_t sgx_total_bytes_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
- return sysfs_emit(buf, "%lu\n", sgx_numa_nodes[dev->id].size);
-}
-static DEVICE_ATTR_RO(sgx_total_bytes);
-
-static struct attribute *arch_node_dev_attrs[] = {
- &dev_attr_sgx_total_bytes.attr,
- NULL,
-};
-
-const struct attribute_group arch_node_dev_group = {
- .name = "x86",
- .attrs = arch_node_dev_attrs,
-};
-#endif /* CONFIG_NUMA */
-
static int __init sgx_init(void)
{
int ret;