netdev - 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]

Message-ID: <20140817124803.GA31996@redhat.com>
Date:	Sun, 17 Aug 2014 14:48:03 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	"Luis R. Rodriguez" <mcgrof@...e.com>,
	"Luis R. Rodriguez" <mcgrof@...not-panic.com>,
	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	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>,
	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 08/17, Oleg Nesterov wrote:
>
> On 08/17, Takashi Iwai wrote:
> >
> > How about just increasing/decreasing the module count for blocking the
> > exit call?  For example:
> >
> > #define module_long_probe_init(initfn)				\
> > 	static int _long_probe_##initfn(void *arg)		\
> > 	{							\
> > 		int ret = initfn();				\
> > 		module_put(THIS_MODULE);			\
>
> WINDOW, please see below.
>
> > 		return ret;					\
> > 	}							\
> > 	static inline __init int __long_probe_##initfn(void)	\
> > 	{							\
> > 		struct task_struct *__init_thread;		\
> > 		__module_get(THIS_MODULE);			\
> > 		__init_thread = kthread_run(_long_probe_##initfn,\
> > 					    NULL,		\
> > 					    #initfn);		\
> > 		if (IS_ERR(__init_thread)) {			\
> > 			module_put(THIS_MODULE);		\
> > 			return PTR_ERR(__init_thread);		\
> > 		}						\
> > 		return 0;					\
> > 	}							\
>
> I leave this to you and Luis, but personally I think this is very
> nice idea, I like it. Because sys_delete_module() won't hang in D
> state waiting for initfn().
>
> There is a small problem. This module can be unloaded right after
> module_put() above. In this case its memory can be unmapped and
> the exiting thread can crash.
>
> This is very unlikely, this thread needs to execute just a few insn
> and escape from this module's memory. Given that only the buggy
> modules should use this hack, perhaps we can even ignore this race.
>
> But perhaps it makes sense to close this race anyway, and we already
> have complete_and_exit() which can be used instead of "return ret"
> above. Just we need the additional "static struct completion" and
> module_exit() should call wait_for_completion.

Forgot to mention... and __long_probe_##initfn() could be simpler
without kthread_run,

	__init_thread = kthread_create(...);
	if (IS_ERR(__init_thread))
		return PTR_ERR();

	module_get(THIS_MODULE);
	wake_up_process(__init_thread);
	return 0;

but this is subjective, up to you.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html