Messages in this thread | | | Subject | Re: [PATCH] [GIT PULL] tracing: allow to change permissions for text with dynamic ftrace enabled | From | Steven Rostedt <> | Date | Wed, 28 Oct 2009 16:15:53 -0400 |
| |
On Wed, 2009-10-28 at 11:56 -0800, Suresh Siddha wrote: > On Tue, 2009-10-27 at 16:31 -0700, Steven Rostedt wrote: > > On Tue, 2009-10-27 at 15:23 -0800, Suresh Siddha wrote: > > > > > > > > > > I'll test it out, and if it does work, you can write up a formal patch > > > > and remove the !define that I added. > > > > > > I just saw one more place calling do_ftrace_mod_code(). So moved this > > > check inside the do_ftrace_mod_code(). Does this cover all the cases? > > > Thanks. > > > > For ftrace, probably. But I just realized this may have other > > consequences. The CPA_DEBUG tests setting a bit in the page table at > > random locations. > > CPA_DEBUG is setting/clearing unused bit 9. It has no interaction with > RW bits. Though the random page and random length selections may end up
Actually it does. When you call the clear_page_attribute, and go to set or clear bit 9, the static_protections() function is called against the new_prot. This will clear the RW bit if it is set, even if the original caller did not touch that bit.
> splitting the large page mappings for kernel text. This is a different > issue that needs to be addressed (perhaps I can ignore this bit 9 as > well for the kernel text mapping. In general it sounds like a bad idea > to break any large page mappings to small page mappings for cpa debug > tests, especially when we don't restore the large page mapping when we > clear those unused bits once the test is done. Will look into this). > > > The code you added makes any change to kernel text > > page attributes remove the RW bit. > > Only for CONFIG_DEBUG_RODATA. > > > On boot up, it is considered that > > kernel text is writable. > > For CONFIG_DEBUG_RODATA, during the boot we change it to RO.
We do that at the end of boot up. I'm sure there's a reason for this :-/
> > > Perhaps you need to add a check in your if statement to only prevent > > that bit when kernel_set_to_readonly is not true. > > Are you worried that kernel_set_to_readonly might not be true but we > might have the kernel text mapping read-only?
Yes, because the code clears the RW bit whenever something changes any attribute of that page table. Including bit 9.
> > As ftrace is now using kernel identity mapping on 64-bit, we can change > the semantics of kernel_set_to_readonly to reflect the status of kernel > identity mapping for 64-bit. I can update the comments.
Well, the identity mapping should still be set to readonly after boot up, right?
-- Steve
| |