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: <Y9kDyUaOe57IL/yF@hirez.programming.kicks-ass.net>
Date:   Tue, 31 Jan 2023 13:04:25 +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 10:58:56AM +0000, Christophe Leroy wrote:
> 
> 
> Le 31/01/2023 à 10:09, Peter Zijlstra a écrit :
> 
> >> @@ -573,23 +574,33 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr);
> >>   bool is_module_percpu_address(unsigned long addr);
> >>   bool is_module_text_address(unsigned long addr);
> >>   
> >> +static inline bool within_module_mem_type(unsigned long addr,
> >> +					  const struct module *mod,
> >> +					  enum mod_mem_type type)
> >> +{
> >> +	unsigned long base, size;
> >> +
> >> +	base = (unsigned long)mod->mem[type].base;
> >> +	size = mod->mem[type].size;
> >> +
> >> +	return base <= addr && addr < base + size;
> > 
> > Possible (as inspired by all the above is_{init,core}() etc..
> > 
> > 	return addr - base < size;
> > 
> 
> In kernel/module/main.c we have a function called within(). Maybe that 
> function could be lifted in module.h and used.

More sharing more good. But I don't think we can lift a 'within'
function to the global namespace, that's just asking for pain.

> > static inline bool within_module_mem_types(unsigned long addr,
> > 					   const struct module *mod,
> > 					   enum mod_mem_type first,
> > 					   enum mod_mem_type last)
> > {
> > 	for (enum mod_mem_type type = first; type <= last; type++) {
> > 		if (within_module_mem_type(addr, mod, type))
> > 			return true;
> > 	}
> > 	return false;
> > }
> 
> Well, ok but what garanties it will always be contiguous types ?
> And you can't anymore see at first look what types it is.
> 
> I prefer it to be explicit with within_module_mem_type(TYPE1) || 
> within_module_mem_type(TYPE2) || within_module_mem_type(TYPE3). By the 
> way we could make the function name shorter, even within() may be a 
> better name as it is used only inside module code.
> 
> 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.