linux-kernel - Re: [PATCH 1/4] module: Add module_subinit{_noexit} and module

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: <68584887-3dec-4ce5-8892-86af50651c41@libero.it>
Date: Thu, 25 Jul 2024 19:14:14 +0200
From: Goffredo Baroncelli <kreijack@...ero.it>
To: Christoph Hellwig <hch@...radead.org>, Arnd Bergmann <arnd@...db.de>
Cc: Youling Tang <youling.tang@...ux.dev>,
 Luis Chamberlain <mcgrof@...nel.org>, Chris Mason <clm@...com>,
 Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>,
 Theodore Ts'o <tytso@....edu>, Andreas Dilger <adilger.kernel@...ger.ca>,
 Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>,
 Linux-Arch <linux-arch@...r.kernel.org>, linux-kernel@...r.kernel.org,
 linux-modules@...r.kernel.org, linux-btrfs@...r.kernel.org,
 linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
 Youling Tang <tangyouling@...inos.cn>
Subject: Re: [PATCH 1/4] module: Add module_subinit{_noexit} and
 module_subeixt helper macros

On 25/07/2024 17.34, Christoph Hellwig wrote:
> On Thu, Jul 25, 2024 at 05:30:58PM +0200, Arnd Bergmann wrote:
>> Now I think we could just make the module_init() macro
>> do the same thing as a built-in initcall() and put
>> an entry in a special section, to let you have multiple
>> entry points in a loadable module.
>>
>> There are still at least two problems though:
>>
>> - while link order is defined between files in a module,
>>    I don't think there is any guarantee for the order between
>>    two initcalls of the same level within a single file.
> 
> I think the sanest answer is to only allow one per file.  If you
> are in the same file anyway calling one function from the other
> is not a big burden.  It really is when they are spread over files
> when it is annoying, and the three examples show that pretty
> clearly.
> 
>> - For built-in code we don't have to worry about matching
>>    the order of the exit calls since they don't exist there.
>>    As I understand, the interesting part of this patch
>>    series is about making sure the order matches between
>>    init and exit, so there still needs to be a way to
>>    express a pair of such calls.
> 
> That's why you want a single macro to define the init and exit
> callbacks, so that the order can be matched up and so that
> error unwinding can use the relative position easily.
> 

Instead of relying to the "expected" order of the compiler/linker,
why doesn't manage the chain explicitly ? Something like:

struct __subexitcall_node {
	void (*exitfn)(void);
	struct subexitcall_node  *next;

static inline void __subexitcall_rollback(struct __subexitcall_node *p)
{
	while (p) {
		p->exitfn();
		p = p->next;
	}
}

#define __subinitcall_noexit(initfn, rollback)					\
do {										\
	int _ret;								\
	_ret = initfn();							\
	if (_ret < 0) {								\
		__subexitcall_rollback(rollback);				\
		return _ret;							\
	}									\
} while (0)

#define __subinitcall(initfn, exitfn, rollback)						\
do {											\
	static subexitcall_node node = {exitfn, rollback->head};	                \
	__subinitcall_noexit(initfn, rollback);						\
	rollback = &node;
} while (0)


#define MODULE_SUBINIT_INIT(rollback) \
	struct __subexitcall_node	*rollback = NULL

#define MODULE_SUBINIT_CALL(initfn, exitfn, rollback) \
	__subinitcall(initfn, exitfn, rollback)

#define MODULE_SUBINIT_CALL_NOEXIT(initfn, rollback) \
	__subinitcall_noexit(initfn, rollback)

#define MODULE_SUBEXIT(rollback) 		\
do {						\
	__subexitcall_rollback(rollback);	\
	rollback = NULL;			\
} while(0)

usage:

	MODULE_SUBINIT_INIT(rollback);

	
	MODULE_SUBINIT_CALL(init_a, exit_a, rollback);
	MODULE_SUBINIT_CALL(init_b, exit_b, rollback);
	MODULE_SUBINIT_CALL_NOEXIT(init_c, rollback);


	MODULE_SUBEXIT(rollback);

this would cost +1 pointer for each function. But this would save from situation like

	r = init_a();
	if (r)
		init_b();
	init_c();




-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5