[PATCH] libnvdimm: Add a NULL entry to 'nvdimm_firmware_attributes'

We recently discovered a kernel oops with 'papr_scm' module while
booting ppc64 phyp guest with following back-trace:

BUG: Kernel NULL pointer dereference on write at 0x00000188
Faulting instruction address: 0xc0000000005d7084
Oops: Kernel access of bad area, sig: 11 [#1]
<snip>
Call Trace:
 internal_create_group+0x128/0x4c0 (unreliable)
 internal_create_groups.part.4+0x70/0x130
 device_add+0x458/0x9c0
 nd_async_device_register+0x28/0xa0 [libnvdimm]
 async_run_entry_fn+0x78/0x1f0
 process_one_work+0x2c0/0x5b0
 worker_thread+0x88/0x650
 kthread+0x1a8/0x1b0
 ret_from_kernel_thread+0x5c/0x6c

A bisect lead to the 'commit 48001ea50d17f ("PM, libnvdimm: Add runtime
firmware activation support")' and on investigation discovered that
the newly introduced 'struct attribute *nvdimm_firmware_attributes[]'
is missing a terminating NULL entry in the array. This causes a loop
in sysfs's 'create_files()' to read garbage beyond bounds of
'nvdimm_firmware_attributes' and trigger the oops.

Fixes: 48001ea50d17f ("PM, libnvdimm: Add runtime firmware activation support")
Reported-by: Sandipan Das <redacted>
Signed-off-by: Vaibhav Jain <redacted>
---
 drivers/nvdimm/dimm_devs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 61374def51555..b59032e0859b7 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c

@@ -529,6 +529,7 @@ static DEVICE_ATTR_ADMIN_RW(activate);
 static struct attribute *nvdimm_firmware_attributes[] = {
 	&dev_attr_activate.attr,
 	&dev_attr_result.attr,
+	NULL,
 };
 
 static umode_t nvdimm_firmware_visible(struct kobject *kobj, struct attribute *a, int n)

-- 
2.26.2

On Fri, Aug 14, 2020 at 08:35:09PM +0530, Vaibhav Jain wrote:

We recently discovered a kernel oops with 'papr_scm' module while
booting ppc64 phyp guest with following back-trace:

BUG: Kernel NULL pointer dereference on write at 0x00000188
Faulting instruction address: 0xc0000000005d7084
Oops: Kernel access of bad area, sig: 11 [#1]
<snip>
Call Trace:
 internal_create_group+0x128/0x4c0 (unreliable)
 internal_create_groups.part.4+0x70/0x130
 device_add+0x458/0x9c0
 nd_async_device_register+0x28/0xa0 [libnvdimm]
 async_run_entry_fn+0x78/0x1f0
 process_one_work+0x2c0/0x5b0
 worker_thread+0x88/0x650
 kthread+0x1a8/0x1b0
 ret_from_kernel_thread+0x5c/0x6c

A bisect lead to the 'commit 48001ea50d17f ("PM, libnvdimm: Add runtime
firmware activation support")' and on investigation discovered that
the newly introduced 'struct attribute *nvdimm_firmware_attributes[]'
is missing a terminating NULL entry in the array. This causes a loop
in sysfs's 'create_files()' to read garbage beyond bounds of
'nvdimm_firmware_attributes' and trigger the oops.

Fixes: 48001ea50d17f ("PM, libnvdimm: Add runtime firmware activation support")
Reported-by: Sandipan Das <redacted>
Signed-off-by: Vaibhav Jain <redacted>

Reviewed-by: Ira Weiny <redacted>

On Fri, 2020-08-14 at 10:10 -0700, Ira Weiny wrote:

On Fri, Aug 14, 2020 at 08:35:09PM +0530, Vaibhav Jain wrote:

quoted

We recently discovered a kernel oops with 'papr_scm' module while
booting ppc64 phyp guest with following back-trace:

BUG: Kernel NULL pointer dereference on write at 0x00000188
Faulting instruction address: 0xc0000000005d7084
Oops: Kernel access of bad area, sig: 11 [#1]
<snip>
Call Trace:
 internal_create_group+0x128/0x4c0 (unreliable)
 internal_create_groups.part.4+0x70/0x130
 device_add+0x458/0x9c0
 nd_async_device_register+0x28/0xa0 [libnvdimm]
 async_run_entry_fn+0x78/0x1f0
 process_one_work+0x2c0/0x5b0
 worker_thread+0x88/0x650
 kthread+0x1a8/0x1b0
 ret_from_kernel_thread+0x5c/0x6c

A bisect lead to the 'commit 48001ea50d17f ("PM, libnvdimm: Add runtime
firmware activation support")' and on investigation discovered that
the newly introduced 'struct attribute *nvdimm_firmware_attributes[]'
is missing a terminating NULL entry in the array. This causes a loop
in sysfs's 'create_files()' to read garbage beyond bounds of
'nvdimm_firmware_attributes' and trigger the oops.

Fixes: 48001ea50d17f ("PM, libnvdimm: Add runtime firmware activation support")
Reported-by: Sandipan Das <redacted>
Signed-off-by: Vaibhav Jain <redacted>

Reviewed-by: Ira Weiny <redacted>

Thanks Vaibhav and Ira. I see this was also reported and fixed by Zqiang
a couple days ago. I'll pick that, merge these trailers and add it to
the fixes queue.

quoted

---
 drivers/nvdimm/dimm_devs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 61374def51555..b59032e0859b7 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c

@@ -529,6 +529,7 @@ static DEVICE_ATTR_ADMIN_RW(activate);
 static struct attribute *nvdimm_firmware_attributes[] = {
 	&dev_attr_activate.attr,
 	&dev_attr_result.attr,
+	NULL,
 };
 
 static umode_t nvdimm_firmware_visible(struct kobject *kobj, struct attribute *a, int n)

-- 
2.26.2

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org