Messages in this thread | | | Date | Tue, 7 Jul 2009 22:06:40 -0700 | From | Arjan van de Ven <> | Subject | Re: [PATCH v4] RO/NX protection for loadable kernel modules |
| |
On Tue, 7 Jul 2009 20:47:42 -0400 Siarhei Liakh <sliakh.lkml@gmail.com> wrote:
> > The patch have been developed for Linux 2.6.30 by Siarhei Liakh > <sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>. > > --- > > Signed-off-by: Siarhei Liakh <sliakh.lkml@gmail.com> > Signed-off-by: Xuxian Jiang <jiang@cs.ncsu.edu>
I like it, you can already put
Acked-by: Arjan van de Ven <arjan@linux.intel.com>
there if you want. If you're going to make a v5 then I do have another suggestion for improvement... (only possible now that the code is very clean)
> + /* Set RO for module text and RO-data*/ > + if (ro_size > 0) { > + begin_addr = (unsigned long) base; > + end_addr = begin_addr + ro_size; > + > + /*skip last page if end address is not page-aligned*/ > + if (!IS_ALIGNED(end_addr, PAGE_SIZE)) > + end_addr = ALIGN(end_addr - PAGE_SIZE,PAGE_SIZE); > + > + /*Set text RO if there are still pages between begin > and end*/ > + if (end_addr > begin_addr) { > + pg_count = PFN_DOWN(end_addr - 1) - > + PFN_DOWN(begin_addr) + 1; > + DEBUGP(" RO: 0x%lx %lu\n", begin_addr, > pg_count); > + set_memory_ro(begin_addr, pg_count); > + } else { > + DEBUGP(" RO: less than a page, not > enforcing.\n"); > + } > + } else { > + DEBUGP(" RO: section not present.\n"); > + }
I *think* this can be done as
begin_pfn = PFN_UP( (base); end_pfn = PFN_DOWN(base + ro_size);
if (end_pfn > begin_pfn) set_memory_ro(begin_pfn >> PAGE_SHIFT, end_pfn - begin_pfn);
(note that I think the +1 you have might be buggy)
if you use PFN_UP/PFN_DOWN like this (rounding the start up, rounding the end down), then your entire "fix alignment" is not needed, the PFN rounding will automatically take case of this.
similar construct also applies to the NX codepath that follows right after this RO codepath.
| |