On 06/18/2007 10:57 AM, Mathieu Desnoyers wrote:
> * Chuck Ebbert ([email protected]) wrote:
>>> + return NOTIFY_STOP;
>>> + }
>>> + return NOTIFY_DONE;
>>> +}
>>> +
>>> +static struct notifier_block immediate_notify = {
>>> + .notifier_call = immediate_notifier,
>>> + .priority = 0x7fffffff, /* we need to be notified first */
>>> +};
>>> +
>>> +/*
>>> + * The address is not aligned. We can only change 1 byte of the value
>>> + * atomically.
>>> + * Must be called with immediate_mutex held.
>>> + */
>>> +int immediate_optimized_set_enable(void *address, char enable)
>>> +{
>>> + char saved_byte;
>>> + int ret;
>>> + char *dest = address;
>>> +
>>> + if (!(enable ^ dest[1])) /* Must be a state change 0<->1 to execute */
>>> + return 0;
>>> +
>>> +#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_PAGEALLOC)
>>> + /* Make sure this page is writable */
>>> + change_page_attr(virt_to_page(address), 1, PAGE_KERNEL_EXEC);
>>> + global_flush_tlb();
>>> +#endif
>> Can't we have a macro or inline to do this, and the setting back
>> to read-only? kprobes also has the same ugly #if blocks...
>>
>> Hmm, what happens if you race with kprobes setting a probe on
>> the same page? Couldn't it unexpectedly become read-only?
>>
>
> Hi Chuck,
>
> I am looking more closely at kprobes; a few comments while we are here:
>
> 1 - Why is kprobe_count an atomic_t variable instead of a simple
> integer? It is always protected by the kprobe_mutex anyway. All this
> synchronization seems redundant.
>
> 2 - I wonder where is the equivalent of my snippet in kprobes code:
>>> +#if defined(CONFIG_DEBUG_RODATA) || defined(CONFIG_DEBUG_PAGEALLOC)
>>> + /* Make sure this page is writable */
>>> + change_page_attr(virt_to_page(address), 1, PAGE_KERNEL_EXEC);
>>> + global_flush_tlb();
>>> +#endif
>
> I fancy it's done by the kprobe_page_fault handler, but I do not see
> clearly how writing the breakpoint from arch_arm_kprobe() in
> non-writeable memory is done.
Looks like it's not merged yet:
http://lkml.org/lkml/2007/6/7/2
This needs to go in before 2.6.22-final
>
> In any case, I would like not to use that kind of approach; I prefer to
> set the memory page to RWX before doing the memory write so I do not
> depend on the page fault handler (remember that I instrument the page
> fault handler itself...).
>
> Maybe we could use a shared "text_mutex" between kprobes and
> immediate values to insure mutual exclusion for .text modification.
> However, we would still have the following coherency issue when an
> immediate value and a kprobe share the same address:
>
> 1- enable immediate value
> 2- put a kprobe at the immediate value load instruction address
> 3- disable immediate value
> 4- remove kprobe
>
> The kprobe removal would put back the load immediate instruction and
> would not touch the loaded value (which is ok). However, the instruction
> copy kept by kprobes would not be in sync with the immediate value
> state:
>
> Scenario 1: kprobes int3 handler first:
>
> 1- enable immediate value
> 2- put a kprobe at the immediate value load instruction address
>
> -> int3 triggered
> kprobe handler runs. Single-steps the "enabled" state.
>
> 3- disable immediate value
>
> -> int3 triggered
> kprobe handler runs. Single-steps the "enabled" state. This state is
> wrong.
>
> 4- remove kprobe
>
>
> Scenario 2: immediate value int3 handler first:
>
> 1- enable immediate value
> 2- put a kprobe at the immediate value load instruction address
>
> -> int3 triggered
> kprobe handler runs. Single-steps the "enabled" state.
>
> 3- disable immediate value
> -> int3 triggered (while we disable)
> While we disable, the immediate value int3 handler is executed first. It
> would cause the kprobe handler not to be called, and no "missing"
> counter would be incremented.
>
> kprobe handler runs. Single-steps the "enabled" state. This state is
> wrong.
>
> 4- remove kprobe
>
>
> Since I don't want to depend on kprobes to put the breakpoint, because
> of its reentrancy limitations (see all the __probes functions), It would
> be good to figure out a mutual exclusion mechanism between immediate
> values and kprobes. Maybe we could forbid kprobes to insert probes on
> addresses present in the immediate values tables ? Or better: if we
> detect this scenario, we could put the kprobe breakpoint at the
> instruction following the "movl".
>
That's up to you and the kprobes people, I guess...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
