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: <20140817122527.GA30546@redhat.com>
Date:	Sun, 17 Aug 2014 14:25:27 +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, 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.

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