linux-kernel - Re: [PATCH v3 1/3] init / kthread: add module_long_probe_init() and module_long_probe

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:	Wed, 13 Aug 2014 09:03:55 +0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	"\"\\\"Luis R. Rodriguez\\\"\"" <mcgrof@...not-panic.com>,
	tiwai@...e.de, linux-kernel@...r.kernel.org,
	"\"\\\"Luis R. Rodriguez\\\"\"" <mcgrof@...e.com>,
	Joseph Salisbury <joseph.salisbury@...onical.com>,
	Kay Sievers <kay@...y.org>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Tim Gardner <tim.gardner@...onical.com>,
	Pierre Fersing <pierre-fersing@...rref.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Benjamin Poirier <bpoirier@...e.de>,
	Nagalakshmi Nandigama <nagalakshmi.nandigama@...gotech.com>,
	Praveen Krishnamoorthy <praveen.krishnamoorthy@...gotech.com>,
	Sreekanth Reddy <sreekanth.reddy@...gotech.com>,
	Abhijit Mahajan <abhijit.mahajan@...gotech.com>,
	Hariprasad S <hariprasad@...lsio.com>,
	Santosh Rastapur <santosh@...lsio.com>,
	MPT-FusionLinux.pdl@...gotech.com, linux-scsi@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH v3 1/3] init / kthread: add module_long_probe_init() and
 module_long_probe_exit()

On Wed, Aug 13, 2014 at 07:59:06AM +0900, Tetsuo Handa wrote:
> Luis R. Rodriguez wrote:
> > Tetsuo bisected and found that commit 786235ee \"kthread: make
> > kthread_create() killable\" modified kthread_create() to bail as
> > soon as SIGKILL is received.
> 
> I just wrote commit 786235ee. It is not Tetsuo who bisected it.
> 
> > @@ -128,4 +129,38 @@ bool queue_kthread_work(struct kthread_worker *worker,
> > void flush_kthread_work(struct kthread_work *work);
> > void flush_kthread_worker(struct kthread_worker *worker);
> > 
> > +#ifndef MODULE
> > +
> > +#define module_long_probe_init(x)      __initcall(x);
> > +#define module_long_probe_exit(x)      __exitcall(x);
> > +
> > +#else
> > +/* To be used by modules which can take over 30 seconds at probe */
> > +#define module_long_probe_init(initfn)                    \\
> > +     static struct task_struct *__init_thread;          \\
> > +     static int _long_probe_##initfn(void *arg)          \\
> > +     {                                   \\
> > +          return initfn();                    \\
> > +     }                                   \\
> > +     static inline __init int __long_probe_##initfn(void)     \\
> > +     {                                   \\
> > +          __init_thread = kthread_run(_long_probe_##initfn,\\
> > +                             NULL,          \\
> > +                             #initfn);          \\
> > +          if (IS_ERR(__init_thread))               \\
> > +               return PTR_ERR(__init_thread);          \\
> > +          return 0;                         \\
> > +     }                                   \\
> > +     module_init(__long_probe_##initfn);
> > +/* To be used by modules that require module_long_probe_init() */
> > +#define module_long_probe_exit(exitfn)                    \\
> > +     static inline void __long_probe_##exitfn(void)          \\
> > +     {                                   \\
> > +          exitfn();                         \\
> 
> exitfn() must not be called if initfn() failed or has not
> completed yet. You need a bool variable for indicating that
> we are ready to call exitfn().
> 
> Also, subsequent userspace operations may fail if
> we return to userspace before initfn() completes
> (e.g. device nodes are not created yet).

I doubt that this will be a problem, as device nodes are usually created
_after_ module_init() returns.

But the cleanup issues are real on error paths.  Given that these
drivers will need "work" anyway, I don't think it's really a big deal.

thanks,

greg k-h
--
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/