linux-kernel - Re: [PATCH v5] module: replace module_layout with 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: <Y9o52aAC33YlRueI@hirez.programming.kicks-ass.net>
Date:   Wed, 1 Feb 2023 11:07:21 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Song Liu <song@...nel.org>
Cc:     linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
        hch@....de, kernel-team@...a.com,
        Luis Chamberlain <mcgrof@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Guenter Roeck <linux@...ck-us.net>,
        Christophe Leroy <christophe.leroy@...roup.eu>
Subject: Re: [PATCH v5] module: replace module_layout with module_memory

On Tue, Jan 31, 2023 at 10:47:20PM -0800, Song Liu wrote:
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 8c5909c0076c..3429d354fec0 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -320,17 +320,50 @@ struct mod_tree_node {
>  	struct latch_tree_node node;
>  };
>  
> +enum mod_mem_type {
	MOD_TEXT = 0,

(just paranoia, and you explicitly rely on that below)

> +	MOD_DATA,
> +	MOD_RODATA,
> +	MOD_RO_AFTER_INIT,
> +	MOD_INIT_TEXT,
> +	MOD_INIT_DATA,
> +	MOD_INIT_RODATA,
> +
> +	MOD_MEM_NUM_TYPES,
> +	MOD_INVALID = -1,
> +};
> +
> +#define mod_mem_type_is_core_data(type)	\
> +	((type) == MOD_DATA ||		\
> +	 (type) == MOD_RODATA ||	\
> +	 (type) == MOD_RO_AFTER_INIT)
> +
> +#define mod_mem_type_is_core(type)	\
> +	((type) == MOD_TEXT ||		\
> +	 mod_mem_type_is_core_data(type))
> +
> +#define mod_mem_type_is_init(type)	\
> +	((type) == MOD_INIT_TEXT ||	\
> +	 (type) == MOD_INIT_DATA ||	\
> +	 (type) == MOD_INIT_RODATA)

Note that, per definition:

  core := !init
  data := !text

(and vice-versa ofcourse, so pick the smallest set) also ISTR you
explicitly needing is_text somewhere.... ah yes, module_enable_nx().

That is; I'd write something like:

#define mod_mem_type_is_core(type) !mod_mem_type_is_init(type)

#define mod_mem_type_is_text(type)	\
	((type) == MOD_TEXT ||		\
	 (type) == MOD_INIT_TEXT)

#define mod_mem_type_is_data(type) !mod_mem_type_is_text(type)

and then possibly additional helpers like is_core_data etc.. where
needed.

#define mod_mem_type_is_core_data(type)	\
	(mod_mem_type_is_core(type) &&	\
	 mod_mem_type_is_data(type))

> +#define for_each_mod_mem_type(type)		\
> +	for (enum mod_mem_type (type) = 0;	\
> +	     (type) < MOD_MEM_NUM_TYPES; (type)++)

So how about instead of this ...

> +#define for_core_mod_mem_type(type)			\
> +	for (enum mod_mem_type (type) = 0;		\
> +	     (type) < MOD_MEM_NUM_TYPES; (type)++)	\
> +		if (mod_mem_type_is_core(type))
> +
> +#define for_init_mod_mem_type(type)			\
> +	for (enum mod_mem_type (type) = 0;		\
> +	     (type) < MOD_MEM_NUM_TYPES; (type)++)	\
> +		if (mod_mem_type_is_init(type))

... you write something like:

#define for_class_mod_mem_type(type, class)		\
	for_each_mod_mem_type(type)			\
		if (mod_mem_type_is_##class(type))

Then we can write things like:

	for_class_mod_mem_type(type, init)
	for_class_mod_mem_type(type, data)

and

	for_class_mod_mem_type(type, core_data)

(this last could be used in show_datasize() for example).

Does that make sense?