Uprobes v3
in [Kernel]
|
Prev: BUG: amd64-agp (2.6.34-rc7)
Next: [RFC] PyTimechart
From: Peter Zijlstra on 11 May 2010 17:10 On Thu, 2010-05-06 at 23:31 +0530, Srikar Dronamraju wrote: > - Addressed comments from Oleg, including removal of interrupt context > handlers, reverting background page replacement in favour of > access_process_vm(). > +static int write_opcode(struct task_struct *tsk, unsigned long vaddr, > + user_bkpt_opcode_t opcode) > +{ > + int ret; > + > + if (!tsk) > + return -EINVAL; > + > + ret = access_process_vm(tsk, vaddr, &opcode, user_bkpt_opcode_sz, 1); > + return (ret == user_bkpt_opcode_sz ? 0 : -EFAULT); > +} Why! That's not not the atomic sequence outlined. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Srikar Dronamraju on 12 May 2010 06:30 * Peter Zijlstra <peterz(a)infradead.org> [2010-05-11 22:59:45]: > On Thu, 2010-05-06 at 23:31 +0530, Srikar Dronamraju wrote: > > - Addressed comments from Oleg, including removal of interrupt context > > handlers, reverting background page replacement in favour of > > access_process_vm(). > > > > +static int write_opcode(struct task_struct *tsk, unsigned long vaddr, > > + user_bkpt_opcode_t opcode) > > +{ > > + int ret; > > + > > + if (!tsk) > > + return -EINVAL; > > + > > + ret = access_process_vm(tsk, vaddr, &opcode, user_bkpt_opcode_sz, 1); > > + return (ret == user_bkpt_opcode_sz ? 0 : -EFAULT); > > +} > > Why! > > That's not not the atomic sequence outlined. > Yes, we had moved away from access_process_vm to background page replacement in Version 1 and Version 2. One of the reasons being Mathieu suggesting to Jim in LFCS that for almost all architectures insertion of a breakpoint instruction on a user page is an atomic operation, as far as the CPU is concerned. Can you and other VM experts tell me if access_process_vm isnt going to be atomic with respect to inserting/deleting a breakpoint instruction? Oleg had few questions which I didnt have answers. (Most of which you have already answered yesterday). One thing that's still missing is [ snipping from Oleg's mail: ] ----- But suppose that the application does mprotect(PROT_WRITE) after register_uprobe() installs the bp, now unregister_uprobe/etc can't restore the original insn? --- Also I tried a write_opcode that uses background page replacement which addressed some of Oleg's comments. The pseudo-code is here: write_opcode() { down_read(mmap_sem); get_user_pages(tsk, mm, vaddr, .. &old_page, &vma); anon_vma_prepare(vma); new_page=alloc_page_vma(.., vma, vaddr); copy_user_page(new_page, old_page); kmap_atomic(new_page,...); memcpy(vaddr,..); kunmap_atomic(..); lock_page(new_page); old_pte = get_pte(mm,vaddr); replace_page(vma, new_page, old_page, old_pte); unlock_page(new_page); put_page(new_page); put_page(old_page); up_read(mmap); } Will this work? The Other VM quieries that I had were: Is there any thing else needed for the parent process to pass on the anon_vma to the child process. (I inserted a breakpoint in the parent and tried removing the breakpoint in the child. However page_address_in_vma() (called by replace_page() returned EFAULT because "vma->anon_vma != page_anon_vma(page)" Do we need to take care of mem_cgroups? Do we need to update mm counters? -- Thanks and Regards Srikar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 12 May 2010 06:40 On Thu, May 06, 2010 at 11:31:39PM +0530, Srikar Dronamraju wrote: > intro.patch > > From: Srikar Dronamraju <srikar(a)linux.vnet.ibm.com> > > Uprobes Patches I can't find the trace event patch inside this set. May be you missed it somehow? (or I did). Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ananth N Mavinakayanahalli on 12 May 2010 09:30 On Wed, May 12, 2010 at 02:13:05PM +0200, Peter Zijlstra wrote: > On Wed, 2010-05-12 at 15:55 +0530, Srikar Dronamraju wrote: > > * Peter Zijlstra <peterz(a)infradead.org> [2010-05-11 22:59:45]: > > > > > On Thu, 2010-05-06 at 23:31 +0530, Srikar Dronamraju wrote: > > > > - Addressed comments from Oleg, including removal of interrupt context > > > > handlers, reverting background page replacement in favour of > > > > access_process_vm(). > > > > > > > > > > +static int write_opcode(struct task_struct *tsk, unsigned long vaddr, > > > > + user_bkpt_opcode_t opcode) > > > > +{ > > > > + int ret; > > > > + > > > > + if (!tsk) > > > > + return -EINVAL; > > > > + > > > > + ret = access_process_vm(tsk, vaddr, &opcode, user_bkpt_opcode_sz, 1); > > > > + return (ret == user_bkpt_opcode_sz ? 0 : -EFAULT); > > > > +} > > > > > > Why! > > > > > > That's not not the atomic sequence outlined. > > > > > > > > > Yes, we had moved away from access_process_vm to background page > > replacement in Version 1 and Version 2. > > > > One of the reasons being Mathieu suggesting to Jim in LFCS that > > for almost all architectures insertion of a breakpoint instruction on a > > user page is an atomic operation, as far as the CPU is concerned. > > That is true, however when restoring the old instruction you do need to > take care, so your usage from set_orig_insn() is dubious. > > > Can you and other VM experts tell me if access_process_vm isnt going to > > be atomic with respect to inserting/deleting a breakpoint instruction? > > Well, clearly not, since it simply does a gup(.force=1), if whatever > page is there is writable it will simply poke at it. > > Now writing the INT3 is special, but restoring the old insn is not and > might confuse another CPU that is currently trying to execute code near > there. Yes, this helps for breakpoint insertion, but... The question is whether only INT3 special or single-byte changes are also guaranteed to be atomic. In http://lkml.org/lkml/2010/1/27/275 Peter Anvin states 'specific case of a more generic rule'. For restoring the old instruction, we technically need to put back just one byte, irrespective of the actual length of the underlying instruction. Now, as long as we have the housekeeping code to handle the possibility of a thread hitting the said breakpoint when its being removed, is it safe to assume atomicity for replacing one byte of possibly a longer instruction? Ananth -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 12 May 2010 09:40
On Wed, 2010-05-12 at 18:57 +0530, Ananth N Mavinakayanahalli wrote: > Now, as long as we have the housekeeping code to handle the > possibility of a thread hitting the said breakpoint when its being > removed, is it safe to assume atomicity for replacing one byte of > possibly a longer instruction? Dunno I'm not a hardware guy, but the issue is so simple to side-step I'm not sure why you're arguing for relying on these special semantics. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |