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

From: Josh Poimboeuf
Date: Mon Nov 09 2015 - 14:51:58 EST


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?

> }
>
> -static void set_section_ro_nx(void *base,
> - unsigned long text_size,
> - unsigned long ro_size,
> - unsigned long total_size,
> - int (*set_ro)(unsigned long start, int num_pages),
> - int (*set_nx)(unsigned long start, int num_pages))
> +static void frob_rodata(const struct module_layout *layout,
> + int (*set_memory)(unsigned long start, int num_pages))
> {
> - /* begin and end PFNs of the current subsection */
> - unsigned long begin_pfn;
> - unsigned long end_pfn;
> -
> - /*
> - * Set RO for module text and RO-data:
> - * - Always protect first page.
> - * - Do not protect last partial page.
> - */
> - if (ro_size > 0)
> - set_page_attributes(base, base + ro_size, set_ro);
> + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
> + BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
> + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
> + set_memory((unsigned long)layout->base + layout->text_size,
> + (layout->ro_size - layout->text_size) >> PAGE_SHIFT);
> +}

Same here, what if ro_size == text_size (no rodata)?

>
> - /*
> - * Set NX permissions for module data:
> - * - Do not protect first partial page.
> - * - Always protect last page.
> - */
> - if (total_size > text_size) {
> - begin_pfn = PFN_UP((unsigned long)base + text_size);
> - end_pfn = PFN_UP((unsigned long)base + total_size);
> - if (end_pfn > begin_pfn)
> - set_nx(begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn);
> - }
> +static void frob_writable_data(const struct module_layout *layout,
> + int (*set_memory)(unsigned long start, int num_pages))
> +{
> + BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
> + BUG_ON((unsigned long)layout->ro_size & (PAGE_SIZE-1));
> + BUG_ON((unsigned long)layout->size & (PAGE_SIZE-1));
> + set_memory((unsigned long)layout->base + layout->ro_size,
> + (layout->size - layout->ro_size) >> PAGE_SHIFT);
> }

Ditto for size == ro_size (no writable data).


--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/