linux-kernel - Re: [patch] driver core: if built-in, do not wait in driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 1 Apr 2007 18:47:48 -0700
From:	Greg KH <gregkh@...e.de>
To:	Ingo Molnar <mingo@...e.hu>, Kay Sievers <kay.sievers@...y.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Pavel Machek <pavel@....cz>, Adrian Bunk <bunk@...sta.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [patch] driver core: if built-in, do not wait in
	driver_unregister()

On Sun, Apr 01, 2007 at 07:35:08PM +0200, Ingo Molnar wrote:
> 
> * Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> > I would suggest that for 2.6.21, the minimal fix is actually something 
> > like the appended. Comments? Ingo, does this fix things for you?
> 
> yeah - it does the trick: i just booted the .config in question and your 
> patch works fine and the bootup does not hang in slgt_init() anymore:
> 
>  Calling initcall 0xc1e78d86: slgt_init+0x0/0x1ee()
>  SyncLink GT $Revision: 4.36 $
>  SyncLink GT no devices found
>  initcall 0xc1e78d86: slgt_init+0x0/0x1ee() returned -19
>  Calling initcall 0xc1e78f74: n_hdlc_init+0x0/0x9c()
>  HDLC line discipline: version $Revision: 4.8 $, maxframe=4096
>  N_HDLC line discipline registered.
>  initcall 0xc1e78f74: n_hdlc_init+0x0/0x9c() returned 0
> 
> thanks! Find below the full patch with metadata filled in (no other 
> changes).

No, I think this will catch the "hang" but look in sysfs in
/sys/modules/ for the module directory for the module that "failed" to
be loaded.  I think you will see some dangling files there that are
incorrect, and might oops if you cat from them (don't remember).

Kay's patch is correct and fixes the reference count issue properly,
this one just papers over it by ignoring the fact that the driver is
never released and cleaned up in memory.

(patch included below so Kay can verify this...)

thanks,

greg k-h

> ------------------------->
> Subject: [patch] driver core: if built-in, do not wait in driver_unregister()
> From: Linus Torvalds <torvalds@...ux-foundation.org>
> 
> built-in drivers suffered bootup hangs with certain driver unregistry
> sequences, due to sysfs breakage.
> 
> do the minimal fix for v2.6.21: only wait if the driver is a module.
> 
> Signed-off-by: Ingo Molnar <mingo@...e.hu>
> ---
>  drivers/base/driver.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Index: linux/drivers/base/driver.c
> ===================================================================
> --- linux.orig/drivers/base/driver.c
> +++ linux/drivers/base/driver.c
> @@ -183,7 +183,14 @@ int driver_register(struct device_driver
>  void driver_unregister(struct device_driver * drv)
>  {
>  	bus_remove_driver(drv);
> -	wait_for_completion(&drv->unloaded);
> +	/*
> +	 * If the driver is a module, we are probably in
> +	 * the module unload path, and we want to wait
> +	 * for everything to unload before we can actually
> +	 * finish the unload.
> +	 */
> +	if (drv->owner)
> +		wait_for_completion(&drv->unloaded);
>  }
>  
>  /**
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/