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]

Message-ID: <2481b9cb-bbe0-fd82-c722-67ba77739669@csgroup.eu>
Date:   Tue, 31 Jan 2023 17:41:12 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Peter Zijlstra <peterz@...radead.org>
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



Le 31/01/2023 à 15:06, Peter Zijlstra a écrit :
> 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.
> 

Well we misunderstood each other then.

I agree with you that category helpers are worth it.

My point was that the implementation of those category helpers need to 
be explicit and not hide types based on some assumption on their order 
in the enum.