Re: [PATCH 2/2] staging: greybus: loopback: fix oops on rmmod gb_loopback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 08, 2017 at 03:27:19PM +0000, Bryan O'Donoghue wrote:
> Doing an rmmod gb_loopback will cause an oops with the following backtrace.
> 
> Call Trace:
>  debugfs_remove+0xaf/0x180
>  gb_loopback_disconnect+0x36/0x160 [gb_loopback]
>  greybus_remove+0x87/0x1a0 [greybus]
>  device_release_driver_internal+0x14a/0x200
>  driver_detach+0x39/0x60
>  bus_remove_driver+0x47/0xa0
>  driver_unregister+0x27/0x50
>  greybus_deregister_driver+0xd/0x10 [greybus]
>  loopback_exit+0x1c/0x36 [gb_loopback]
>  SyS_delete_module+0x170/0x1b0
>  entry_SYSCALL_64_fastpath+0x13/0x93
> 
> This happens because gb_dev.root is removed prior to
> greybus_deregister_driver which itself will trigger gb_loopback_disconnect
> to run - leading to the parent of gb->file being NULL.
> 
> Typically up to this point when tearing down greybus modules we have
> removed gb_es2 prior to removing gb_loopback. In this case gb_es2 will run
> subordinate greybus->disconnect() routines. A subsequent rmmod of
> gb_loopback then will not re-run gb_loopback_disconnect().
> 
> On project Ara the second module removal flow was the only practiced flow
> hence it's only now in gbsim where we have more freedom to rmmod that this
> bug is apparent. The fix itself is pretty straight-forward.

There was nothing preventing you from unloading bundle-drivers while
keeping the host-device driver (e.g. gb_es2) loaded on Project Ara, even
if this typically wasn't done, yes.

> ---
>  drivers/staging/greybus/loopback.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 88d4f5c..8f3eb33 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -1260,9 +1260,9 @@ module_init(loopback_init);
>  
>  static void __exit loopback_exit(void)
>  {
> -	debugfs_remove_recursive(gb_dev.root);
>  	greybus_deregister(&gb_loopback_driver);
>  	class_unregister(&loopback_class);
> +	debugfs_remove_recursive(gb_dev.root);

This is just a plain bug, and in fact, you should only be using
debugfs_remove here, as any bundle-specific files should have been
cleaned up as part of disconnect.

>  	ida_destroy(&loopback_ida);
>  }
>  module_exit(loopback_exit);

Thanks,
Johan
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux