Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 12:34:50PM +0200, Alexandre Belloni wrote: > On 16/08/2018 12:04:13+0200, Greg Kroah-Hartman wrote: > > On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > > > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > > > When both uio and the uio drivers are built in the kernel, it is > > > > > possible > > > > > for a driver to register devices before the uio class is registered. > > > > > > > > How does this happen? The link order should solve this issue, right? > > > > > > > > > > Sure, if we can ensure uio_init() is called before any driver calls > > > uio_register_device() then this would not happen. However, I'm not sure > > > how you would want to achieve that. > > > > That is the job of the link order, does this not work properly today? > > How have you triggered this so that you could test your patch? > > > > I have a (not yet upstreamed) MFD driver in drivers/mfd that is > registering the uio device. It mostly look like uio_pci_generic.c. > > When compiling both as builtin, it will crash that way. > > There are no link dependency between uio_init and uio_register_device > calls so I guess the linker can't reorder that properly. Ah, ok, yes, that will break, I was looking at the kernel tree _today_ :) I'll queue this up after 4.19-rc1 is out, thanks. greg k-h
Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 12:34:50PM +0200, Alexandre Belloni wrote: > On 16/08/2018 12:04:13+0200, Greg Kroah-Hartman wrote: > > On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > > > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > > > When both uio and the uio drivers are built in the kernel, it is > > > > > possible > > > > > for a driver to register devices before the uio class is registered. > > > > > > > > How does this happen? The link order should solve this issue, right? > > > > > > > > > > Sure, if we can ensure uio_init() is called before any driver calls > > > uio_register_device() then this would not happen. However, I'm not sure > > > how you would want to achieve that. > > > > That is the job of the link order, does this not work properly today? > > How have you triggered this so that you could test your patch? > > > > I have a (not yet upstreamed) MFD driver in drivers/mfd that is > registering the uio device. It mostly look like uio_pci_generic.c. > > When compiling both as builtin, it will crash that way. > > There are no link dependency between uio_init and uio_register_device > calls so I guess the linker can't reorder that properly. Ah, ok, yes, that will break, I was looking at the kernel tree _today_ :) I'll queue this up after 4.19-rc1 is out, thanks. greg k-h
Re: [PATCH v2] uio: ensure class is registered before devices
On 16/08/2018 12:04:13+0200, Greg Kroah-Hartman wrote: > On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > > When both uio and the uio drivers are built in the kernel, it is > > > > possible > > > > for a driver to register devices before the uio class is registered. > > > > > > How does this happen? The link order should solve this issue, right? > > > > > > > Sure, if we can ensure uio_init() is called before any driver calls > > uio_register_device() then this would not happen. However, I'm not sure > > how you would want to achieve that. > > That is the job of the link order, does this not work properly today? > How have you triggered this so that you could test your patch? > I have a (not yet upstreamed) MFD driver in drivers/mfd that is registering the uio device. It mostly look like uio_pci_generic.c. When compiling both as builtin, it will crash that way. There are no link dependency between uio_init and uio_register_device calls so I guess the linker can't reorder that properly. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2] uio: ensure class is registered before devices
On 16/08/2018 12:04:13+0200, Greg Kroah-Hartman wrote: > On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > > When both uio and the uio drivers are built in the kernel, it is > > > > possible > > > > for a driver to register devices before the uio class is registered. > > > > > > How does this happen? The link order should solve this issue, right? > > > > > > > Sure, if we can ensure uio_init() is called before any driver calls > > uio_register_device() then this would not happen. However, I'm not sure > > how you would want to achieve that. > > That is the job of the link order, does this not work properly today? > How have you triggered this so that you could test your patch? > I have a (not yet upstreamed) MFD driver in drivers/mfd that is registering the uio device. It mostly look like uio_pci_generic.c. When compiling both as builtin, it will crash that way. There are no link dependency between uio_init and uio_register_device calls so I guess the linker can't reorder that properly. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > When both uio and the uio drivers are built in the kernel, it is possible > > > for a driver to register devices before the uio class is registered. > > > > How does this happen? The link order should solve this issue, right? > > > > Sure, if we can ensure uio_init() is called before any driver calls > uio_register_device() then this would not happen. However, I'm not sure > how you would want to achieve that. That is the job of the link order, does this not work properly today? How have you triggered this so that you could test your patch? thanks, greg k-h
Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 10:34:38AM +0200, Alexandre Belloni wrote: > On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > > When both uio and the uio drivers are built in the kernel, it is possible > > > for a driver to register devices before the uio class is registered. > > > > How does this happen? The link order should solve this issue, right? > > > > Sure, if we can ensure uio_init() is called before any driver calls > uio_register_device() then this would not happen. However, I'm not sure > how you would want to achieve that. That is the job of the link order, does this not work properly today? How have you triggered this so that you could test your patch? thanks, greg k-h
Re: [PATCH v2] uio: ensure class is registered before devices
On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > When both uio and the uio drivers are built in the kernel, it is possible > > for a driver to register devices before the uio class is registered. > > How does this happen? The link order should solve this issue, right? > Sure, if we can ensure uio_init() is called before any driver calls uio_register_device() then this would not happen. However, I'm not sure how you would want to achieve that. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2] uio: ensure class is registered before devices
On 16/08/2018 10:01:12+0200, Greg Kroah-Hartman wrote: > On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > > When both uio and the uio drivers are built in the kernel, it is possible > > for a driver to register devices before the uio class is registered. > > How does this happen? The link order should solve this issue, right? > Sure, if we can ensure uio_init() is called before any driver calls uio_register_device() then this would not happen. However, I'm not sure how you would want to achieve that. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > When both uio and the uio drivers are built in the kernel, it is possible > for a driver to register devices before the uio class is registered. How does this happen? The link order should solve this issue, right? thanks, greg k-h
Re: [PATCH v2] uio: ensure class is registered before devices
On Thu, Aug 16, 2018 at 09:39:41AM +0200, Alexandre Belloni wrote: > When both uio and the uio drivers are built in the kernel, it is possible > for a driver to register devices before the uio class is registered. How does this happen? The link order should solve this issue, right? thanks, greg k-h
[PATCH v2] uio: ensure class is registered before devices
When both uio and the uio drivers are built in the kernel, it is possible
for a driver to register devices before the uio class is registered.
This may result in a NULL pointer dereference later on in
get_device_parent() when accessing the class glue_dirs spinlock.
The trace looks like that:
Unable to handle kernel NULL pointer dereference at virtual address 0140
[...]
[] _raw_spin_lock+0x14/0x48
[] device_add+0x154/0x6a0
[] device_create_groups_vargs+0x120/0x128
[] device_create+0x54/0x60
[] __uio_register_device+0x120/0x4a8
[] jaguar2_pci_probe+0x2d4/0x558
[] local_pci_probe+0x3c/0xb8
[] pci_device_probe+0x11c/0x180
[] driver_probe_device+0x22c/0x2d8
[] __driver_attach+0xbc/0xc0
[] bus_for_each_dev+0x4c/0x98
[] driver_attach+0x20/0x28
[] bus_add_driver+0x1b8/0x228
[] driver_register+0x60/0xf8
[] __pci_register_driver+0x40/0x48
Return EPROBE_DEFER in that case so the driver can register the device
later.
Signed-off-by: Alexandre Belloni
---
drivers/uio/uio.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5d421d7e8904..55b523f7499b 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -275,6 +275,8 @@ static struct class uio_class = {
.dev_groups = uio_groups,
};
+bool uio_class_registered;
+
/*
* device functions
*/
@@ -884,6 +886,9 @@ static int init_uio_class(void)
printk(KERN_ERR "class_register failed for uio\n");
goto err_class_register;
}
+
+ uio_class_registered = true;
+
return 0;
err_class_register:
@@ -894,6 +899,7 @@ static int init_uio_class(void)
static void release_uio_class(void)
{
+ uio_class_registered = false;
class_unregister(_class);
uio_major_cleanup();
}
@@ -920,6 +926,9 @@ int __uio_register_device(struct module *owner,
struct uio_device *idev;
int ret = 0;
+ if (!uio_class_registered)
+ return -EPROBE_DEFER;
+
if (!parent || !info || !info->name || !info->version)
return -EINVAL;
--
2.18.0
[PATCH v2] uio: ensure class is registered before devices
When both uio and the uio drivers are built in the kernel, it is possible
for a driver to register devices before the uio class is registered.
This may result in a NULL pointer dereference later on in
get_device_parent() when accessing the class glue_dirs spinlock.
The trace looks like that:
Unable to handle kernel NULL pointer dereference at virtual address 0140
[...]
[] _raw_spin_lock+0x14/0x48
[] device_add+0x154/0x6a0
[] device_create_groups_vargs+0x120/0x128
[] device_create+0x54/0x60
[] __uio_register_device+0x120/0x4a8
[] jaguar2_pci_probe+0x2d4/0x558
[] local_pci_probe+0x3c/0xb8
[] pci_device_probe+0x11c/0x180
[] driver_probe_device+0x22c/0x2d8
[] __driver_attach+0xbc/0xc0
[] bus_for_each_dev+0x4c/0x98
[] driver_attach+0x20/0x28
[] bus_add_driver+0x1b8/0x228
[] driver_register+0x60/0xf8
[] __pci_register_driver+0x40/0x48
Return EPROBE_DEFER in that case so the driver can register the device
later.
Signed-off-by: Alexandre Belloni
---
drivers/uio/uio.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 5d421d7e8904..55b523f7499b 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -275,6 +275,8 @@ static struct class uio_class = {
.dev_groups = uio_groups,
};
+bool uio_class_registered;
+
/*
* device functions
*/
@@ -884,6 +886,9 @@ static int init_uio_class(void)
printk(KERN_ERR "class_register failed for uio\n");
goto err_class_register;
}
+
+ uio_class_registered = true;
+
return 0;
err_class_register:
@@ -894,6 +899,7 @@ static int init_uio_class(void)
static void release_uio_class(void)
{
+ uio_class_registered = false;
class_unregister(_class);
uio_major_cleanup();
}
@@ -920,6 +926,9 @@ int __uio_register_device(struct module *owner,
struct uio_device *idev;
int ret = 0;
+ if (!uio_class_registered)
+ return -EPROBE_DEFER;
+
if (!parent || !info || !info->name || !info->version)
return -EINVAL;
--
2.18.0
