Messages in this thread Patch in this message | | | From | Rusty Russell <> | Subject | Re: [PATCH v4] RO/NX protection for loadable kernel modules | Date | Sat, 11 Jul 2009 21:19:45 +0930 |
| |
On Thu, 9 Jul 2009 08:01:59 am Siarhei Liakh wrote: > [ Arjen wrote: ] > > 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) > > The +1 is necessary because of (end_addr - 1). However, I like your > way better, so I will incorporate it in V5.
In fact, this wasn't quite what you did. You did begin_pfn = PFN_DOWN(), end_pfn = PFN_DOWN(), then later use PFN_UP on both of them.
Is this what you intended?
(Note: I removed some DEBUGPs, where action is implied anyway).
diff --git a/kernel/module.c b/kernel/module.c --- a/kernel/module.c +++ b/kernel/module.c @@ -1517,55 +1517,41 @@ static void set_section_ro_nx(void *base unsigned long begin_pfn; unsigned long end_pfn; +#ifdef CONFIG_DEBUG_RODATA + /* Most module_alloc use vmalloc which page-aligns. If not, + * we could be missing protection on first part of module. */ + WARN_ON(offset_in_page((unsigned long)base)); +#endif + /* Initially, all module sections have RWX permissions*/ - DEBUGP("PROTECTING MODULE SECTION: 0x%lx\n" " text size: %lu\n" " ro size: %lu\n" " total size: %lu\n", (unsigned long)base, - text_size, ro_size, total_size); + text_size, ro_size, total_size); - /* Set RO for module text and RO-data*/ - if (ro_size > 0) { - /* Always protect first page. - * Do not protect last partial page */ - begin_pfn = PFN_DOWN((unsigned long)base); - end_pfn = PFN_DOWN((unsigned long)base + ro_size); + /* Set RO for module text and RO-data */ + /* Don't protect partial pages. */ + begin_pfn = PFN_UP((unsigned long)base); + end_pfn = PFN_DOWN((unsigned long)base + ro_size); - /*Set text RO if there are still pages between begin and end*/ - if (end_pfn > begin_pfn) { - DEBUGP(" RO: 0x%lx %lu\n", - begin_pfn << PAGE_SHIFT, - end_pfn - begin_pfn); - set_memory_ro(begin_pfn << PAGE_SHIFT, - end_pfn - begin_pfn); - } else { - DEBUGP(" RO: less than a page, not enforcing.\n"); - } - } else { - DEBUGP(" RO: section not present.\n"); + /* Set text RO if there are still pages between begin and end */ + if (end_pfn > begin_pfn) { + DEBUGP(" RO: 0x%lx %lu\n", + begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn); + set_memory_ro(begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn); } /* Set NX permissions for module data */ - if (total_size > text_size) { - /* Do not protect first partial page - * Always protect last page. */ - begin_pfn = PFN_UP((unsigned long)base + text_size); - end_pfn = PFN_UP((unsigned long)base + total_size); + /* Don't protect partial pages. */ + begin_pfn = PFN_UP((unsigned long)base + text_size); + end_pfn = PFN_DOWN((unsigned long)base + total_size); - /*Set data NX if there are still pages between begin and end*/ - if (end_pfn > begin_pfn) { - DEBUGP(" NX: 0x%lx %lu\n", - begin_pfn << PAGE_SHIFT, - end_pfn - begin_pfn); - set_memory_nx(begin_pfn << PAGE_SHIFT, - end_pfn - begin_pfn); - } else { - DEBUGP(" NX: less than a page, not enforcing.\n"); - } - } else { - DEBUGP(" NX: section not present.\n"); + if (end_pfn > begin_pfn) { + DEBUGP(" NX: 0x%lx %lu\n", + begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn); + set_memory_nx(begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn); } }
| |