All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
@ 2021-02-09 22:17 Dave Jiang
  2021-02-10  6:44 ` Greg KH
  2021-02-10 13:05   ` kernel test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Dave Jiang @ 2021-02-09 22:17 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(). Call the auxiliary_bus_init()
under driver_init() so the bus is initialized 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>
---

v2:
- Call in driver_init() to ensure aux bus gets init before devices.  (GregKH)

 drivers/base/auxiliary.c |   10 +---------
 drivers/base/base.h      |    5 +++++
 drivers/base/init.c      |    1 +
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index 8336535f1e11..8ff389653126 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -260,19 +260,11 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
 }
 EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
 
-static int __init auxiliary_bus_init(void)
+int __init auxiliary_bus_init(void)
 {
 	return bus_register(&auxiliary_bus_type);
 }
 
-static void __exit auxiliary_bus_exit(void)
-{
-	bus_unregister(&auxiliary_bus_type);
-}
-
-module_init(auxiliary_bus_init);
-module_exit(auxiliary_bus_exit);
-
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Auxiliary Bus");
 MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>");
diff --git a/drivers/base/base.h b/drivers/base/base.h
index f5600a83124f..978ad265c42e 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -119,6 +119,11 @@ static inline int hypervisor_init(void) { return 0; }
 extern int platform_bus_init(void);
 extern void cpu_dev_init(void);
 extern void container_dev_init(void);
+#ifdef CONFIG_AUXILIARY_BUS
+extern int auxiliary_bus_init(void);
+#else
+static inline int auxiliary_bus_init(void) { return 0; }
+#endif
 
 struct kobject *virtual_device_parent(struct device *dev);
 
diff --git a/drivers/base/init.c b/drivers/base/init.c
index 908e6520e804..a9f57c22fb9e 100644
--- a/drivers/base/init.c
+++ b/drivers/base/init.c
@@ -32,6 +32,7 @@ void __init driver_init(void)
 	 */
 	of_core_init();
 	platform_bus_init();
+	auxiliary_bus_init();
 	cpu_dev_init();
 	memory_dev_init();
 	container_dev_init();



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
  2021-02-09 22:17 [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init Dave Jiang
@ 2021-02-10  6:44 ` Greg KH
  2021-02-10 13:05   ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2021-02-10  6:44 UTC (permalink / raw)
  To: Dave Jiang; +Cc: Jacob Pan, Dave Ertman, Dan Williams, rafael, linux-kernel

On Tue, Feb 09, 2021 at 03:17:00PM -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(). Call the auxiliary_bus_init()
> under driver_init() so the bus is initialized 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>
> ---
> 
> v2:
> - Call in driver_init() to ensure aux bus gets init before devices.  (GregKH)
> 
>  drivers/base/auxiliary.c |   10 +---------
>  drivers/base/base.h      |    5 +++++
>  drivers/base/init.c      |    1 +
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index 8336535f1e11..8ff389653126 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -260,19 +260,11 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv)
>  }
>  EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
>  
> -static int __init auxiliary_bus_init(void)
> +int __init auxiliary_bus_init(void)
>  {
>  	return bus_register(&auxiliary_bus_type);

Ok, you return an int, and then...

>  }
>  
> -static void __exit auxiliary_bus_exit(void)
> -{
> -	bus_unregister(&auxiliary_bus_type);
> -}
> -
> -module_init(auxiliary_bus_init);
> -module_exit(auxiliary_bus_exit);
> -
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Auxiliary Bus");
>  MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>");
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index f5600a83124f..978ad265c42e 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -119,6 +119,11 @@ static inline int hypervisor_init(void) { return 0; }
>  extern int platform_bus_init(void);
>  extern void cpu_dev_init(void);
>  extern void container_dev_init(void);
> +#ifdef CONFIG_AUXILIARY_BUS
> +extern int auxiliary_bus_init(void);
> +#else
> +static inline int auxiliary_bus_init(void) { return 0; }
> +#endif
>  
>  struct kobject *virtual_device_parent(struct device *dev);
>  
> diff --git a/drivers/base/init.c b/drivers/base/init.c
> index 908e6520e804..a9f57c22fb9e 100644
> --- a/drivers/base/init.c
> +++ b/drivers/base/init.c
> @@ -32,6 +32,7 @@ void __init driver_init(void)
>  	 */
>  	of_core_init();
>  	platform_bus_init();
> +	auxiliary_bus_init();

Ignore it :(

Please just make the function not return anything.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
  2021-02-09 22:17 [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init Dave Jiang
@ 2021-02-10 13:05   ` kernel test robot
  2021-02-10 13:05   ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-02-10 13:05 UTC (permalink / raw)
  To: Dave Jiang, gregkh
  Cc: kbuild-all, clang-built-linux, Jacob Pan, Dave Ertman,
	Dan Williams, rafael, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

Hi Dave,

I love your patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on linux/master linus/master v5.11-rc7 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dave-Jiang/driver-core-auxiliary-bus-Fix-calling-stage-for-auxiliary-bus-init/20210210-090304
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 3c9ea42802a1fbf7ef29660ff8c6e526c58114f6
config: arm64-randconfig-r012-20210209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/97ee533f0e048844cd7804bbd0fac219a76b8bf5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Jiang/driver-core-auxiliary-bus-Fix-calling-stage-for-auxiliary-bus-init/20210210-090304
        git checkout 97ee533f0e048844cd7804bbd0fac219a76b8bf5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/base/auxiliary.c:263:12: warning: no previous prototype for function 'auxiliary_bus_init' [-Wmissing-prototypes]
   int __init auxiliary_bus_init(void)
              ^
   drivers/base/auxiliary.c:263:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __init auxiliary_bus_init(void)
   ^
   static 
   1 warning generated.


vim +/auxiliary_bus_init +263 drivers/base/auxiliary.c

   262	
 > 263	int __init auxiliary_bus_init(void)
   264	{
   265		return bus_register(&auxiliary_bus_type);
   266	}
   267	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39575 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init
@ 2021-02-10 13:05   ` kernel test robot
  0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-02-10 13:05 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2499 bytes --]

Hi Dave,

I love your patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on linux/master linus/master v5.11-rc7 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dave-Jiang/driver-core-auxiliary-bus-Fix-calling-stage-for-auxiliary-bus-init/20210210-090304
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 3c9ea42802a1fbf7ef29660ff8c6e526c58114f6
config: arm64-randconfig-r012-20210209 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/97ee533f0e048844cd7804bbd0fac219a76b8bf5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dave-Jiang/driver-core-auxiliary-bus-Fix-calling-stage-for-auxiliary-bus-init/20210210-090304
        git checkout 97ee533f0e048844cd7804bbd0fac219a76b8bf5
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/base/auxiliary.c:263:12: warning: no previous prototype for function 'auxiliary_bus_init' [-Wmissing-prototypes]
   int __init auxiliary_bus_init(void)
              ^
   drivers/base/auxiliary.c:263:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __init auxiliary_bus_init(void)
   ^
   static 
   1 warning generated.


vim +/auxiliary_bus_init +263 drivers/base/auxiliary.c

   262	
 > 263	int __init auxiliary_bus_init(void)
   264	{
   265		return bus_register(&auxiliary_bus_type);
   266	}
   267	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39575 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-10 13:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 22:17 [PATCH v2] driver core: auxiliary bus: Fix calling stage for auxiliary bus init Dave Jiang
2021-02-10  6:44 ` Greg KH
2021-02-10 13:05 ` kernel test robot
2021-02-10 13:05   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.