* [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
@ 2021-02-09 19:05 Dave Jiang
2021-02-09 19:14 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jiang @ 2021-02-09 19:05 UTC (permalink / raw)
To: gregkh; +Cc: Jacob Pan, Dave Ertman, Dan Williams, rafael, linux-kernel
When the auxiliary device code is built into the kernel, it can be executed
before the auxiliary bus is registered. This causes bus->p to be not
allocated and triggers a NULL pointer dereference when the auxiliary bus
device gets added with bus_add_device(). Change the init of auxiliary bus
to subsys_initcall() from module_init() to ensure the bus is registered
before devices.
Below is the kernel splat for the bug:
[ 1.948215] BUG: kernel NULL pointer dereference, address: 0000000000000060
[ 1.950670] #PF: supervisor read access in kernel mode
[ 1.950670] #PF: error_code(0x0000) - not-present page
[ 1.950670] PGD 0
[ 1.950670] Oops: 0000 1 SMP NOPTI
[ 1.950670] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-intel-nextsvmtest+ #2205
[ 1.950670] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 1.950670] RIP: 0010:bus_add_device+0x64/0x140
[ 1.950670] Code: 00 49 8b 75 20 48 89 df e8 59 a1 ff ff 41 89 c4 85 c0 75 7b 48 8b 53 50 48 85 d2 75 03 48 8b 13 49 8b 85 a0 00 00 00 48 89 de <48> 8
78 60 48 83 c7 18 e8 ef d9 a9 ff 41 89 c4 85 c0 75 45 48 8b
[ 1.950670] RSP: 0000:ff46032ac001baf8 EFLAGS: 00010246
[ 1.950670] RAX: 0000000000000000 RBX: ff4597f7414aa680 RCX: 0000000000000000
[ 1.950670] RDX: ff4597f74142bbc0 RSI: ff4597f7414aa680 RDI: ff4597f7414aa680
[ 1.950670] RBP: ff46032ac001bb10 R08: 0000000000000044 R09: 0000000000000228
[ 1.950670] R10: ff4597f741141b30 R11: ff4597f740182a90 R12: 0000000000000000
[ 1.950670] R13: ffffffffa5e936c0 R14: 0000000000000000 R15: 0000000000000000
[ 1.950670] FS: 0000000000000000(0000) GS:ff4597f7bba00000(0000) knlGS:0000000000000000
[ 1.950670] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1.950670] CR2: 0000000000000060 CR3: 000000002140c001 CR4: 0000000000f71ef0
[ 1.950670] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1.950670] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[ 1.950670] PKRU: 55555554
[ 1.950670] Call Trace:
[ 1.950670] device_add+0x3ee/0x850
[ 1.950670] __auxiliary_device_add+0x47/0x60
[ 1.950670] idxd_pci_probe+0xf77/0x1180
[ 1.950670] local_pci_probe+0x4a/0x90
[ 1.950670] pci_device_probe+0xff/0x1b0
[ 1.950670] really_probe+0x1cf/0x440
[ 1.950670] ? rdinit_setup+0x31/0x31
[ 1.950670] driver_probe_device+0xe8/0x150
[ 1.950670] device_driver_attach+0x58/0x60
[ 1.950670] __driver_attach+0x8f/0x150
[ 1.950670] ? device_driver_attach+0x60/0x60
[ 1.950670] ? device_driver_attach+0x60/0x60
[ 1.950670] bus_for_each_dev+0x79/0xc0
[ 1.950670] ? kmem_cache_alloc_trace+0x323/0x430
[ 1.950670] driver_attach+0x1e/0x20
[ 1.950670] bus_add_driver+0x154/0x1f0
[ 1.950670] driver_register+0x70/0xc0
[ 1.950670] __pci_register_driver+0x54/0x60
[ 1.950670] idxd_init_module+0xe2/0xfc
[ 1.950670] ? idma64_platform_driver_init+0x19/0x19
[ 1.950670] do_one_initcall+0x4a/0x1e0
[ 1.950670] kernel_init_freeable+0x1fc/0x25c
[ 1.950670] ? rest_init+0xba/0xba
[ 1.950670] kernel_init+0xe/0x116
[ 1.950670] ret_from_fork+0x1f/0x30
[ 1.950670] Modules linked in:
[ 1.950670] CR2: 0000000000000060
[ 1.950670] --[ end trace cd7d1b226d3ca901 ]--
Fixes: 7de3697e9cbd ("Add auxiliary bus support")
Reported-by: Jacob Pan <jacob.jun.pan@intel.com>
Acked-by: Dave Ertman <david.m.ertman@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/base/auxiliary.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index 8336535f1e11..53f93a506626 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
bus_unregister(&auxiliary_bus_type);
}
-module_init(auxiliary_bus_init);
+subsys_initcall(auxiliary_bus_init);
module_exit(auxiliary_bus_exit);
MODULE_LICENSE("GPL v2");
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
2021-02-09 19:05 [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init Dave Jiang
@ 2021-02-09 19:14 ` Greg KH
2021-02-09 19:38 ` Dave Jiang
2021-02-09 20:01 ` Dan Williams
0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2021-02-09 19:14 UTC (permalink / raw)
To: Dave Jiang; +Cc: Jacob Pan, Dave Ertman, Dan Williams, rafael, linux-kernel
On Tue, Feb 09, 2021 at 12:05:05PM -0700, Dave Jiang wrote:
> When the auxiliary device code is built into the kernel, it can be executed
> before the auxiliary bus is registered. This causes bus->p to be not
> allocated and triggers a NULL pointer dereference when the auxiliary bus
> device gets added with bus_add_device(). Change the init of auxiliary bus
> to subsys_initcall() from module_init() to ensure the bus is registered
> before devices.
>
> Below is the kernel splat for the bug:
> [ 1.948215] BUG: kernel NULL pointer dereference, address: 0000000000000060
> [ 1.950670] #PF: supervisor read access in kernel mode
> [ 1.950670] #PF: error_code(0x0000) - not-present page
> [ 1.950670] PGD 0
> [ 1.950670] Oops: 0000 1 SMP NOPTI
> [ 1.950670] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-intel-nextsvmtest+ #2205
> [ 1.950670] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [ 1.950670] RIP: 0010:bus_add_device+0x64/0x140
> [ 1.950670] Code: 00 49 8b 75 20 48 89 df e8 59 a1 ff ff 41 89 c4 85 c0 75 7b 48 8b 53 50 48 85 d2 75 03 48 8b 13 49 8b 85 a0 00 00 00 48 89 de <48> 8
> 78 60 48 83 c7 18 e8 ef d9 a9 ff 41 89 c4 85 c0 75 45 48 8b
> [ 1.950670] RSP: 0000:ff46032ac001baf8 EFLAGS: 00010246
> [ 1.950670] RAX: 0000000000000000 RBX: ff4597f7414aa680 RCX: 0000000000000000
> [ 1.950670] RDX: ff4597f74142bbc0 RSI: ff4597f7414aa680 RDI: ff4597f7414aa680
> [ 1.950670] RBP: ff46032ac001bb10 R08: 0000000000000044 R09: 0000000000000228
> [ 1.950670] R10: ff4597f741141b30 R11: ff4597f740182a90 R12: 0000000000000000
> [ 1.950670] R13: ffffffffa5e936c0 R14: 0000000000000000 R15: 0000000000000000
> [ 1.950670] FS: 0000000000000000(0000) GS:ff4597f7bba00000(0000) knlGS:0000000000000000
> [ 1.950670] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1.950670] CR2: 0000000000000060 CR3: 000000002140c001 CR4: 0000000000f71ef0
> [ 1.950670] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1.950670] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
> [ 1.950670] PKRU: 55555554
> [ 1.950670] Call Trace:
> [ 1.950670] device_add+0x3ee/0x850
> [ 1.950670] __auxiliary_device_add+0x47/0x60
> [ 1.950670] idxd_pci_probe+0xf77/0x1180
> [ 1.950670] local_pci_probe+0x4a/0x90
> [ 1.950670] pci_device_probe+0xff/0x1b0
> [ 1.950670] really_probe+0x1cf/0x440
> [ 1.950670] ? rdinit_setup+0x31/0x31
> [ 1.950670] driver_probe_device+0xe8/0x150
> [ 1.950670] device_driver_attach+0x58/0x60
> [ 1.950670] __driver_attach+0x8f/0x150
> [ 1.950670] ? device_driver_attach+0x60/0x60
> [ 1.950670] ? device_driver_attach+0x60/0x60
> [ 1.950670] bus_for_each_dev+0x79/0xc0
> [ 1.950670] ? kmem_cache_alloc_trace+0x323/0x430
> [ 1.950670] driver_attach+0x1e/0x20
> [ 1.950670] bus_add_driver+0x154/0x1f0
> [ 1.950670] driver_register+0x70/0xc0
> [ 1.950670] __pci_register_driver+0x54/0x60
> [ 1.950670] idxd_init_module+0xe2/0xfc
> [ 1.950670] ? idma64_platform_driver_init+0x19/0x19
> [ 1.950670] do_one_initcall+0x4a/0x1e0
> [ 1.950670] kernel_init_freeable+0x1fc/0x25c
> [ 1.950670] ? rest_init+0xba/0xba
> [ 1.950670] kernel_init+0xe/0x116
> [ 1.950670] ret_from_fork+0x1f/0x30
> [ 1.950670] Modules linked in:
> [ 1.950670] CR2: 0000000000000060
> [ 1.950670] --[ end trace cd7d1b226d3ca901 ]--
>
> Fixes: 7de3697e9cbd ("Add auxiliary bus support")
> Reported-by: Jacob Pan <jacob.jun.pan@intel.com>
> Acked-by: Dave Ertman <david.m.ertman@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/base/auxiliary.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index 8336535f1e11..53f93a506626 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
> bus_unregister(&auxiliary_bus_type);
> }
>
> -module_init(auxiliary_bus_init);
> +subsys_initcall(auxiliary_bus_init);
Ah, the linker priority dance. Are you _SURE_ this will solve this?
Why not just call this explicitly in driver_init() so that you know it
will be ok? Just like we do for the platform bus?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
2021-02-09 19:14 ` Greg KH
@ 2021-02-09 19:38 ` Dave Jiang
2021-02-09 19:55 ` Greg KH
2021-02-09 20:01 ` Dan Williams
1 sibling, 1 reply; 6+ messages in thread
From: Dave Jiang @ 2021-02-09 19:38 UTC (permalink / raw)
To: Greg KH; +Cc: Jacob Pan, Dave Ertman, Dan Williams, rafael, linux-kernel
On 2/9/2021 12:14 PM, Greg KH wrote:
> On Tue, Feb 09, 2021 at 12:05:05PM -0700, Dave Jiang wrote:
>> When the auxiliary device code is built into the kernel, it can be executed
>> before the auxiliary bus is registered. This causes bus->p to be not
>> allocated and triggers a NULL pointer dereference when the auxiliary bus
>> device gets added with bus_add_device(). Change the init of auxiliary bus
>> to subsys_initcall() from module_init() to ensure the bus is registered
>> before devices.
>>
>> Below is the kernel splat for the bug:
>> [ 1.948215] BUG: kernel NULL pointer dereference, address: 0000000000000060
>> [ 1.950670] #PF: supervisor read access in kernel mode
>> [ 1.950670] #PF: error_code(0x0000) - not-present page
>> [ 1.950670] PGD 0
>> [ 1.950670] Oops: 0000 1 SMP NOPTI
>> [ 1.950670] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-intel-nextsvmtest+ #2205
>> [ 1.950670] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>> [ 1.950670] RIP: 0010:bus_add_device+0x64/0x140
>> [ 1.950670] Code: 00 49 8b 75 20 48 89 df e8 59 a1 ff ff 41 89 c4 85 c0 75 7b 48 8b 53 50 48 85 d2 75 03 48 8b 13 49 8b 85 a0 00 00 00 48 89 de <48> 8
>> 78 60 48 83 c7 18 e8 ef d9 a9 ff 41 89 c4 85 c0 75 45 48 8b
>> [ 1.950670] RSP: 0000:ff46032ac001baf8 EFLAGS: 00010246
>> [ 1.950670] RAX: 0000000000000000 RBX: ff4597f7414aa680 RCX: 0000000000000000
>> [ 1.950670] RDX: ff4597f74142bbc0 RSI: ff4597f7414aa680 RDI: ff4597f7414aa680
>> [ 1.950670] RBP: ff46032ac001bb10 R08: 0000000000000044 R09: 0000000000000228
>> [ 1.950670] R10: ff4597f741141b30 R11: ff4597f740182a90 R12: 0000000000000000
>> [ 1.950670] R13: ffffffffa5e936c0 R14: 0000000000000000 R15: 0000000000000000
>> [ 1.950670] FS: 0000000000000000(0000) GS:ff4597f7bba00000(0000) knlGS:0000000000000000
>> [ 1.950670] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 1.950670] CR2: 0000000000000060 CR3: 000000002140c001 CR4: 0000000000f71ef0
>> [ 1.950670] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 1.950670] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
>> [ 1.950670] PKRU: 55555554
>> [ 1.950670] Call Trace:
>> [ 1.950670] device_add+0x3ee/0x850
>> [ 1.950670] __auxiliary_device_add+0x47/0x60
>> [ 1.950670] idxd_pci_probe+0xf77/0x1180
>> [ 1.950670] local_pci_probe+0x4a/0x90
>> [ 1.950670] pci_device_probe+0xff/0x1b0
>> [ 1.950670] really_probe+0x1cf/0x440
>> [ 1.950670] ? rdinit_setup+0x31/0x31
>> [ 1.950670] driver_probe_device+0xe8/0x150
>> [ 1.950670] device_driver_attach+0x58/0x60
>> [ 1.950670] __driver_attach+0x8f/0x150
>> [ 1.950670] ? device_driver_attach+0x60/0x60
>> [ 1.950670] ? device_driver_attach+0x60/0x60
>> [ 1.950670] bus_for_each_dev+0x79/0xc0
>> [ 1.950670] ? kmem_cache_alloc_trace+0x323/0x430
>> [ 1.950670] driver_attach+0x1e/0x20
>> [ 1.950670] bus_add_driver+0x154/0x1f0
>> [ 1.950670] driver_register+0x70/0xc0
>> [ 1.950670] __pci_register_driver+0x54/0x60
>> [ 1.950670] idxd_init_module+0xe2/0xfc
>> [ 1.950670] ? idma64_platform_driver_init+0x19/0x19
>> [ 1.950670] do_one_initcall+0x4a/0x1e0
>> [ 1.950670] kernel_init_freeable+0x1fc/0x25c
>> [ 1.950670] ? rest_init+0xba/0xba
>> [ 1.950670] kernel_init+0xe/0x116
>> [ 1.950670] ret_from_fork+0x1f/0x30
>> [ 1.950670] Modules linked in:
>> [ 1.950670] CR2: 0000000000000060
>> [ 1.950670] --[ end trace cd7d1b226d3ca901 ]--
>>
>> Fixes: 7de3697e9cbd ("Add auxiliary bus support")
>> Reported-by: Jacob Pan <jacob.jun.pan@intel.com>
>> Acked-by: Dave Ertman <david.m.ertman@intel.com>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> drivers/base/auxiliary.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
>> index 8336535f1e11..53f93a506626 100644
>> --- a/drivers/base/auxiliary.c
>> +++ b/drivers/base/auxiliary.c
>> @@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
>> bus_unregister(&auxiliary_bus_type);
>> }
>>
>> -module_init(auxiliary_bus_init);
>> +subsys_initcall(auxiliary_bus_init);
> Ah, the linker priority dance. Are you _SURE_ this will solve this?
>
> Why not just call this explicitly in driver_init() so that you know it
> will be ok? Just like we do for the platform bus?
It would require making auxiliary_bus part of the base just like
platform instead of a module. Is that ok?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
2021-02-09 19:38 ` Dave Jiang
@ 2021-02-09 19:55 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-02-09 19:55 UTC (permalink / raw)
To: Dave Jiang; +Cc: Jacob Pan, Dave Ertman, Dan Williams, rafael, linux-kernel
On Tue, Feb 09, 2021 at 12:38:06PM -0700, Dave Jiang wrote:
>
> On 2/9/2021 12:14 PM, Greg KH wrote:
> > On Tue, Feb 09, 2021 at 12:05:05PM -0700, Dave Jiang wrote:
> > > When the auxiliary device code is built into the kernel, it can be executed
> > > before the auxiliary bus is registered. This causes bus->p to be not
> > > allocated and triggers a NULL pointer dereference when the auxiliary bus
> > > device gets added with bus_add_device(). Change the init of auxiliary bus
> > > to subsys_initcall() from module_init() to ensure the bus is registered
> > > before devices.
> > >
> > > Below is the kernel splat for the bug:
> > > [ 1.948215] BUG: kernel NULL pointer dereference, address: 0000000000000060
> > > [ 1.950670] #PF: supervisor read access in kernel mode
> > > [ 1.950670] #PF: error_code(0x0000) - not-present page
> > > [ 1.950670] PGD 0
> > > [ 1.950670] Oops: 0000 1 SMP NOPTI
> > > [ 1.950670] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-intel-nextsvmtest+ #2205
> > > [ 1.950670] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> > > [ 1.950670] RIP: 0010:bus_add_device+0x64/0x140
> > > [ 1.950670] Code: 00 49 8b 75 20 48 89 df e8 59 a1 ff ff 41 89 c4 85 c0 75 7b 48 8b 53 50 48 85 d2 75 03 48 8b 13 49 8b 85 a0 00 00 00 48 89 de <48> 8
> > > 78 60 48 83 c7 18 e8 ef d9 a9 ff 41 89 c4 85 c0 75 45 48 8b
> > > [ 1.950670] RSP: 0000:ff46032ac001baf8 EFLAGS: 00010246
> > > [ 1.950670] RAX: 0000000000000000 RBX: ff4597f7414aa680 RCX: 0000000000000000
> > > [ 1.950670] RDX: ff4597f74142bbc0 RSI: ff4597f7414aa680 RDI: ff4597f7414aa680
> > > [ 1.950670] RBP: ff46032ac001bb10 R08: 0000000000000044 R09: 0000000000000228
> > > [ 1.950670] R10: ff4597f741141b30 R11: ff4597f740182a90 R12: 0000000000000000
> > > [ 1.950670] R13: ffffffffa5e936c0 R14: 0000000000000000 R15: 0000000000000000
> > > [ 1.950670] FS: 0000000000000000(0000) GS:ff4597f7bba00000(0000) knlGS:0000000000000000
> > > [ 1.950670] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [ 1.950670] CR2: 0000000000000060 CR3: 000000002140c001 CR4: 0000000000f71ef0
> > > [ 1.950670] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > [ 1.950670] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
> > > [ 1.950670] PKRU: 55555554
> > > [ 1.950670] Call Trace:
> > > [ 1.950670] device_add+0x3ee/0x850
> > > [ 1.950670] __auxiliary_device_add+0x47/0x60
> > > [ 1.950670] idxd_pci_probe+0xf77/0x1180
> > > [ 1.950670] local_pci_probe+0x4a/0x90
> > > [ 1.950670] pci_device_probe+0xff/0x1b0
> > > [ 1.950670] really_probe+0x1cf/0x440
> > > [ 1.950670] ? rdinit_setup+0x31/0x31
> > > [ 1.950670] driver_probe_device+0xe8/0x150
> > > [ 1.950670] device_driver_attach+0x58/0x60
> > > [ 1.950670] __driver_attach+0x8f/0x150
> > > [ 1.950670] ? device_driver_attach+0x60/0x60
> > > [ 1.950670] ? device_driver_attach+0x60/0x60
> > > [ 1.950670] bus_for_each_dev+0x79/0xc0
> > > [ 1.950670] ? kmem_cache_alloc_trace+0x323/0x430
> > > [ 1.950670] driver_attach+0x1e/0x20
> > > [ 1.950670] bus_add_driver+0x154/0x1f0
> > > [ 1.950670] driver_register+0x70/0xc0
> > > [ 1.950670] __pci_register_driver+0x54/0x60
> > > [ 1.950670] idxd_init_module+0xe2/0xfc
> > > [ 1.950670] ? idma64_platform_driver_init+0x19/0x19
> > > [ 1.950670] do_one_initcall+0x4a/0x1e0
> > > [ 1.950670] kernel_init_freeable+0x1fc/0x25c
> > > [ 1.950670] ? rest_init+0xba/0xba
> > > [ 1.950670] kernel_init+0xe/0x116
> > > [ 1.950670] ret_from_fork+0x1f/0x30
> > > [ 1.950670] Modules linked in:
> > > [ 1.950670] CR2: 0000000000000060
> > > [ 1.950670] --[ end trace cd7d1b226d3ca901 ]--
> > >
> > > Fixes: 7de3697e9cbd ("Add auxiliary bus support")
> > > Reported-by: Jacob Pan <jacob.jun.pan@intel.com>
> > > Acked-by: Dave Ertman <david.m.ertman@intel.com>
> > > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > > ---
> > > drivers/base/auxiliary.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> > > index 8336535f1e11..53f93a506626 100644
> > > --- a/drivers/base/auxiliary.c
> > > +++ b/drivers/base/auxiliary.c
> > > @@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
> > > bus_unregister(&auxiliary_bus_type);
> > > }
> > > -module_init(auxiliary_bus_init);
> > > +subsys_initcall(auxiliary_bus_init);
> > Ah, the linker priority dance. Are you _SURE_ this will solve this?
> >
> > Why not just call this explicitly in driver_init() so that you know it
> > will be ok? Just like we do for the platform bus?
>
> It would require making auxiliary_bus part of the base just like platform
> instead of a module.
The auxiliary_bus code is not a module today, so I have no idea what
you are talking about, sorry.
> Is that ok?
No it is not.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
2021-02-09 19:14 ` Greg KH
2021-02-09 19:38 ` Dave Jiang
@ 2021-02-09 20:01 ` Dan Williams
2021-02-10 6:44 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Dan Williams @ 2021-02-09 20:01 UTC (permalink / raw)
To: Greg KH
Cc: Dave Jiang, Jacob Pan, Dave Ertman, Rafael J. Wysocki,
Linux Kernel Mailing List
On Tue, Feb 9, 2021 at 11:16 AM Greg KH <gregkh@linuxfoundation.org> wrote:
[..]
> > diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> > index 8336535f1e11..53f93a506626 100644
> > --- a/drivers/base/auxiliary.c
> > +++ b/drivers/base/auxiliary.c
> > @@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
> > bus_unregister(&auxiliary_bus_type);
> > }
> >
> > -module_init(auxiliary_bus_init);
> > +subsys_initcall(auxiliary_bus_init);
>
> Ah, the linker priority dance. Are you _SURE_ this will solve this?
All users are module_init() today so it will work.... today. The
moment someone wants to use it in a built-in driver that uses
subsystem_init() it will ultimately be chased into driver_init().
> Why not just call this explicitly in driver_init() so that you know it
> will be ok? Just like we do for the platform bus?
Cross that bridge when / if it happens?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
2021-02-09 20:01 ` Dan Williams
@ 2021-02-10 6:44 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-02-10 6:44 UTC (permalink / raw)
To: Dan Williams
Cc: Dave Jiang, Jacob Pan, Dave Ertman, Rafael J. Wysocki,
Linux Kernel Mailing List
On Tue, Feb 09, 2021 at 12:01:06PM -0800, Dan Williams wrote:
> On Tue, Feb 9, 2021 at 11:16 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> [..]
> > > diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> > > index 8336535f1e11..53f93a506626 100644
> > > --- a/drivers/base/auxiliary.c
> > > +++ b/drivers/base/auxiliary.c
> > > @@ -270,7 +270,7 @@ static void __exit auxiliary_bus_exit(void)
> > > bus_unregister(&auxiliary_bus_type);
> > > }
> > >
> > > -module_init(auxiliary_bus_init);
> > > +subsys_initcall(auxiliary_bus_init);
> >
> > Ah, the linker priority dance. Are you _SURE_ this will solve this?
>
> All users are module_init() today so it will work.... today. The
> moment someone wants to use it in a built-in driver that uses
> subsystem_init() it will ultimately be chased into driver_init().
>
> > Why not just call this explicitly in driver_init() so that you know it
> > will be ok? Just like we do for the platform bus?
>
> Cross that bridge when / if it happens?
Let's fix it properly now please.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-10 6:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 19:05 [PATCH] driver core: auxiliary bus: Fix calling stage for auxiliary bus init Dave Jiang
2021-02-09 19:14 ` Greg KH
2021-02-09 19:38 ` Dave Jiang
2021-02-09 19:55 ` Greg KH
2021-02-09 20:01 ` Dan Williams
2021-02-10 6:44 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).