Re: [PATCH 4/4] module: clean up RO/NX handling.

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Tue, Nov 10, 2015 at 12:27:34PM +1030, Rusty Russell wrote:
> Josh Poimboeuf <jpoimboe@redhat.com> writes:
> > On Mon, Nov 09, 2015 at 02:53:57PM +1030, Rusty Russell wrote:
> >
> >> @@ -1858,74 +1849,75 @@ static void mod_sysfs_teardown(struct module *mod)
> >>  /*
> >>   * LKM RO/NX protection: protect module's text/ro-data
> >>   * from modification and any data from execution.
> >> + *
> >> + * General layout of module is:
> >> + *          [text] [read-only-data] [writable data]
> >> + * text_size -----^                ^               ^
> >> + * ro_size ------------------------|               |
> >> + * size -------------------------------------------|
> >> + *
> >> + * These values are always page-aligned (as is base)
> >>   */
> >> -void set_page_attributes(void *start, void *end, int (*set)(unsigned long start, int num_pages))
> >> +static void frob_text(const struct module_layout *layout,
> >> +		      int (*set_memory)(unsigned long start, int num_pages))
> >>  {
> >> -	unsigned long begin_pfn = PFN_DOWN((unsigned long)start);
> >> -	unsigned long end_pfn = PFN_DOWN((unsigned long)end);
> >> -
> >> -	if (end_pfn > begin_pfn)
> >> -		set(begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn);
> >> +	BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
> >> +	BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
> >> +	set_memory((unsigned long)layout->base,
> >> +		   layout->text_size >> PAGE_SHIFT);
> >
> > Should the set_memory() call be skipped if text_size is 0?
> 
> Not AFAICT.  And in practice:
> 1) Every module on my system has a .text section.
> 2) Every module has a rodata section (.modinfo)
> 3) Every module on my system has a .data section.
> 
> So I think it would be a premature optimization.

However, the frob functions are also used for init sections.

A search on my Fedora system's modules for .init.* sections shows that
most modules don't have .init.rodata and .init.data, and some modules
don't even have .init.text.

-- 
Josh