On Thu, Nov 5, 2015 at 5:05 PM, Laura Abbott <labbott@xxxxxxxxxx> wrote: > On 11/05/2015 08:27 AM, Russell King - ARM Linux wrote: >> >> On Thu, Nov 05, 2015 at 08:20:42AM -0800, Laura Abbott wrote: >>> >>> On 11/05/2015 01:46 AM, Russell King - ARM Linux wrote: >>>> >>>> On Wed, Nov 04, 2015 at 05:00:39PM -0800, Laura Abbott wrote: >>>>> >>>>> Currently, read only permissions are not being applied even >>>>> when CONFIG_DEBUG_RODATA is set. This is because section_update >>>>> uses current->mm for adjusting the page tables. current->mm >>>>> need not be equivalent to the kernel version. Use pgd_offset_k >>>>> to get the proper page directory for updating. >>>> >>>> >>>> What are you trying to achieve here? You can't use these functions >>>> at run time (after the first thread has been spawned) to change >>>> permissions, because there will be multiple copies of the kernel >>>> section mappings, and those copies will not get updated. >>>> >>>> In any case, this change will probably break kexec and ftrace, as >>>> the running thread will no longer see the updated page tables. >>>> >>> >>> I think I was hitting that exact problem with multiple copies >>> not getting updated. The section_update code was being called >>> and I was seeing the tables get updated but nothing was being >>> applied when I tried to write to text or check the debugfs >>> page table. The current flow is: >>> >>> rest_init -> kernel_thread(kernel_init) and from that thread >>> mark_rodata_ro. So mark_rodata_ro is always going to happen >>> in a thread. >>> >>> Do we need to update for both init_mm and the first running >>> thread? >> >> >> The "first running thread" is merely coincidental for things like kexec. >> >> Hmm. Actually, I think the existing code _should_ be fine. At the >> point where mark_rodata_ro() is, we should still be using init_mm, so >> updating the current threads page tables should actually be updating >> the swapper_pg_dir. > > > That doesn't seem to hold true. Based on what I'm seeing, we lose > the the guarantee of init_mm after the first exec. If usermodehelper > gets called to load a module, that triggers an exec and the kernel > thread is no longer using init_mm after that. I'm testing with the > multi-v7 defconfig which uses the smsc911x driver which loads a > module during initcall. That gets called before mark_rodata_ro so > the init_mm is never updated. I verified that disabling smsc911x > makes things work as expected. I suspect the testing was never done > with a driver that tried to call usermodehelper during init time. Ooooh. Nice catch. Yeah, my testing didn't include that case. > I got as far as narrowing it down that it happens after the usermodehelper > but I wasn't able to pinpoint where exactly the switch happened. It seems > like we need to have the page tables set up before any initcalls > happen otherwise we risk having an exec create stray processes which we > can't update. Can we just make mark_rodata_ro() a no-op and do the RO setting earlier when we do the NX setting? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>