linux-kernel - Re: [PATCH v4] 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]

Date:   Tue, 31 Jan 2023 15:06:38 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Song Liu <song@...nel.org>,
        "linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "hch@....de" <hch@....de>,
        "kernel-team@...a.com" <kernel-team@...a.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH v4] module: replace module_layout with module_memory

On Tue, Jan 31, 2023 at 12:14:48PM +0000, Christophe Leroy wrote:

> >> Something like
> >>
> >> 	return within(addr, mod, MOD_TEXT) || within(addr, mod, MOD_DATA) ||
> >> 	       within(addr, mod, MOD_RODATA) || within(addr, mod,
> >> MOD_RO_AFTER_INIT);
> > 
> > Urgh, how about?
> > 
> > 	for_each_mod_mem_type(type) {
> > 		if (!mod_mem_type_is_init(type) && within(addr, mod, type))
> > 			return true;
> > 	}
> > 	return false;
> > 
> > Then you have have a bunch of mod_mem_type_id_foo() filter functions
> > that are non-contiguous without having to endlessly repeat stuff
> > manually.
> 
> But that's un-readable.

"For all except init."

> You have to have the list of possible types in front of you in order to 
> understand what the function does. Which means that one day or another 
> someone will change the order of types in the enum, and it will break.

I really don't agree, if you do explicit type lists everywhere you have
to update each and every sites when you modify the enum.

If you make category helpers, like: data, text, init, then you only need
to update the helpers without having to worry about each site. Only if
you add an enum that doesn't fit the existing categories do you need to
do something new.