From: Ravi Bangoria <ravi.bangoria@linux.ibm.com> To: srikar@linux.vnet.ibm.com, oleg@redhat.com, rostedt@goodmis.org, mhiramat@kernel.org Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com, jolsa@redhat.com, namhyung@kernel.org, linux-kernel@vger.kernel.org, ananth@linux.vnet.ibm.com, alexis.berlemont@gmail.com, naveen.n.rao@linux.vnet.ibm.com, linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org, linux@armlinux.org.uk, ralf@linux-mips.org, paul.burton@mips.com, Ravi Bangoria <ravi.bangoria@linux.ibm.com> Subject: [PATCH v6 5/6] Uprobes/sdt: Prevent multiple reference counter for same uprobe Date: Mon, 16 Jul 2018 14:17:05 +0530 [thread overview] Message-ID: <20180716084706.28244-6-ravi.bangoria@linux.ibm.com> (raw) In-Reply-To: <20180716084706.28244-1-ravi.bangoria@linux.ibm.com> We assume to have only one reference counter for one uprobe. Don't allow user to register multiple uprobes having same inode+offset but different reference counter. Though, existing tools which already support SDT events creates normal uprobe and updates reference counter on their own. Allow 0 as a special value for reference counter offset. I.e. two uprobes, one having ref_ctr_offset=0 and the other having non-zero ref_ctr_offset can coexists. This gives user a flexibility to either depend on kernel uprobe infrastructure to maintain reference counter or just use normal uprobe and maintain reference counter on his own. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- kernel/events/uprobes.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 84da8512a974..563cc3e625b3 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -63,6 +63,8 @@ static struct percpu_rw_semaphore dup_mmap_sem; /* Have a copy of original instruction */ #define UPROBE_COPY_INSN 0 +/* Reference counter offset is reloaded with non-zero value. */ +#define REF_CTR_OFF_RELOADED 1 struct uprobe { struct rb_node rb_node; /* node in the rb tree */ @@ -476,9 +478,23 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, return ret; ret = verify_opcode(old_page, vaddr, &opcode); - if (ret <= 0) + if (ret < 0) goto put_old; + /* + * If instruction is already patched but reference counter offset + * has been reloaded to non-zero value, increment the reference + * counter and return. + */ + if (ret == 0) { + if (is_register && + test_bit(REF_CTR_OFF_RELOADED, &uprobe->flags)) { + WARN_ON(!uprobe->ref_ctr_offset); + ret = update_ref_ctr(uprobe, mm, true); + } + goto put_old; + } + /* We are going to replace instruction, update ref_ctr. */ if (!ref_ctr_updated && uprobe->ref_ctr_offset) { ret = update_ref_ctr(uprobe, mm, is_register); @@ -679,6 +695,30 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset, cur_uprobe = insert_uprobe(uprobe); /* a uprobe exists for this inode:offset combination */ if (cur_uprobe) { + /* + * If inode+offset matches, ref_ctr_offset must match as + * well. Though, 0 is a special value for ref_ctr_offset. + */ + if (cur_uprobe->ref_ctr_offset != uprobe->ref_ctr_offset && + cur_uprobe->ref_ctr_offset != 0 && + uprobe->ref_ctr_offset != 0) { + pr_warn("Err: Reference counter mismatch.\n"); + put_uprobe(cur_uprobe); + kfree(uprobe); + return ERR_PTR(-EINVAL); + } + + /* + * If existing uprobe->ref_ctr_offset is 0 and user is + * registering same uprobe with non-zero ref_ctr_offset, + * set new ref_ctr_offset to existing uprobe. + */ + + if (!cur_uprobe->ref_ctr_offset && uprobe->ref_ctr_offset) { + cur_uprobe->ref_ctr_offset = uprobe->ref_ctr_offset; + set_bit(REF_CTR_OFF_RELOADED, &cur_uprobe->flags); + } + kfree(uprobe); uprobe = cur_uprobe; } @@ -971,6 +1011,7 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) bool is_register = !!new; struct map_info *info; int err = 0; + bool installed = false; percpu_down_write(&dup_mmap_sem); info = build_map_info(uprobe->inode->i_mapping, @@ -1000,8 +1041,10 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) if (is_register) { /* consult only the "caller", new consumer. */ if (consumer_filter(new, - UPROBE_FILTER_REGISTER, mm)) + UPROBE_FILTER_REGISTER, mm)) { err = install_breakpoint(uprobe, mm, vma, info->vaddr); + installed = true; + } } else if (test_bit(MMF_HAS_UPROBES, &mm->flags)) { if (!filter_chain(uprobe, UPROBE_FILTER_UNREGISTER, mm)) @@ -1016,6 +1059,8 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) } out: percpu_up_write(&dup_mmap_sem); + if (installed) + clear_bit(REF_CTR_OFF_RELOADED, &uprobe->flags); return err; } @@ -1093,6 +1138,9 @@ static int __uprobe_register(struct inode *inode, loff_t offset, uprobe = alloc_uprobe(inode, offset, ref_ctr_offset); if (!uprobe) return -ENOMEM; + if (IS_ERR(uprobe)) + return PTR_ERR(uprobe); + /* * We can race with uprobe_unregister()->delete_uprobe(). * Check uprobe_is_active() and retry if it is false. -- 2.14.4
WARNING: multiple messages have this Message-ID (diff)
From: ravi.bangoria@linux.ibm.com (Ravi Bangoria) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v6 5/6] Uprobes/sdt: Prevent multiple reference counter for same uprobe Date: Mon, 16 Jul 2018 14:17:05 +0530 [thread overview] Message-ID: <20180716084706.28244-6-ravi.bangoria@linux.ibm.com> (raw) In-Reply-To: <20180716084706.28244-1-ravi.bangoria@linux.ibm.com> We assume to have only one reference counter for one uprobe. Don't allow user to register multiple uprobes having same inode+offset but different reference counter. Though, existing tools which already support SDT events creates normal uprobe and updates reference counter on their own. Allow 0 as a special value for reference counter offset. I.e. two uprobes, one having ref_ctr_offset=0 and the other having non-zero ref_ctr_offset can coexists. This gives user a flexibility to either depend on kernel uprobe infrastructure to maintain reference counter or just use normal uprobe and maintain reference counter on his own. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- kernel/events/uprobes.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 84da8512a974..563cc3e625b3 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -63,6 +63,8 @@ static struct percpu_rw_semaphore dup_mmap_sem; /* Have a copy of original instruction */ #define UPROBE_COPY_INSN 0 +/* Reference counter offset is reloaded with non-zero value. */ +#define REF_CTR_OFF_RELOADED 1 struct uprobe { struct rb_node rb_node; /* node in the rb tree */ @@ -476,9 +478,23 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, return ret; ret = verify_opcode(old_page, vaddr, &opcode); - if (ret <= 0) + if (ret < 0) goto put_old; + /* + * If instruction is already patched but reference counter offset + * has been reloaded to non-zero value, increment the reference + * counter and return. + */ + if (ret == 0) { + if (is_register && + test_bit(REF_CTR_OFF_RELOADED, &uprobe->flags)) { + WARN_ON(!uprobe->ref_ctr_offset); + ret = update_ref_ctr(uprobe, mm, true); + } + goto put_old; + } + /* We are going to replace instruction, update ref_ctr. */ if (!ref_ctr_updated && uprobe->ref_ctr_offset) { ret = update_ref_ctr(uprobe, mm, is_register); @@ -679,6 +695,30 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset, cur_uprobe = insert_uprobe(uprobe); /* a uprobe exists for this inode:offset combination */ if (cur_uprobe) { + /* + * If inode+offset matches, ref_ctr_offset must match as + * well. Though, 0 is a special value for ref_ctr_offset. + */ + if (cur_uprobe->ref_ctr_offset != uprobe->ref_ctr_offset && + cur_uprobe->ref_ctr_offset != 0 && + uprobe->ref_ctr_offset != 0) { + pr_warn("Err: Reference counter mismatch.\n"); + put_uprobe(cur_uprobe); + kfree(uprobe); + return ERR_PTR(-EINVAL); + } + + /* + * If existing uprobe->ref_ctr_offset is 0 and user is + * registering same uprobe with non-zero ref_ctr_offset, + * set new ref_ctr_offset to existing uprobe. + */ + + if (!cur_uprobe->ref_ctr_offset && uprobe->ref_ctr_offset) { + cur_uprobe->ref_ctr_offset = uprobe->ref_ctr_offset; + set_bit(REF_CTR_OFF_RELOADED, &cur_uprobe->flags); + } + kfree(uprobe); uprobe = cur_uprobe; } @@ -971,6 +1011,7 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) bool is_register = !!new; struct map_info *info; int err = 0; + bool installed = false; percpu_down_write(&dup_mmap_sem); info = build_map_info(uprobe->inode->i_mapping, @@ -1000,8 +1041,10 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) if (is_register) { /* consult only the "caller", new consumer. */ if (consumer_filter(new, - UPROBE_FILTER_REGISTER, mm)) + UPROBE_FILTER_REGISTER, mm)) { err = install_breakpoint(uprobe, mm, vma, info->vaddr); + installed = true; + } } else if (test_bit(MMF_HAS_UPROBES, &mm->flags)) { if (!filter_chain(uprobe, UPROBE_FILTER_UNREGISTER, mm)) @@ -1016,6 +1059,8 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) } out: percpu_up_write(&dup_mmap_sem); + if (installed) + clear_bit(REF_CTR_OFF_RELOADED, &uprobe->flags); return err; } @@ -1093,6 +1138,9 @@ static int __uprobe_register(struct inode *inode, loff_t offset, uprobe = alloc_uprobe(inode, offset, ref_ctr_offset); if (!uprobe) return -ENOMEM; + if (IS_ERR(uprobe)) + return PTR_ERR(uprobe); + /* * We can race with uprobe_unregister()->delete_uprobe(). * Check uprobe_is_active() and retry if it is false. -- 2.14.4
next prev parent reply other threads:[~2018-07-16 8:47 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-16 8:47 [PATCH v6 0/6] Uprobes: Support SDT markers having reference count (semaphore) Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-16 8:47 ` [PATCH v6 1/6] Uprobes: Simplify uprobe_register() body Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-16 8:47 ` [PATCH v6 2/6] Uprobe: Additional argument arch_uprobe to uprobe_write_opcode() Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-16 8:47 ` [PATCH v6 3/6] Uprobes: Support SDT markers having reference count (semaphore) Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-23 16:26 ` Oleg Nesterov 2018-07-23 16:26 ` Oleg Nesterov 2018-07-24 3:34 ` Ravi Bangoria 2018-07-24 3:34 ` Ravi Bangoria 2018-07-27 13:59 ` Oleg Nesterov 2018-07-27 13:59 ` Oleg Nesterov 2018-07-24 14:21 ` Masami Hiramatsu 2018-07-24 14:21 ` Masami Hiramatsu 2018-07-16 8:47 ` [PATCH v6 4/6] trace_uprobe/sdt: Prevent multiple reference counter for same uprobe Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria [this message] 2018-07-16 8:47 ` [PATCH v6 5/6] Uprobes/sdt: " Ravi Bangoria 2018-07-25 11:08 ` Oleg Nesterov 2018-07-25 11:08 ` Oleg Nesterov 2018-07-27 4:17 ` Ravi Bangoria 2018-07-27 4:17 ` Ravi Bangoria 2018-07-27 13:55 ` Oleg Nesterov 2018-07-27 13:55 ` Oleg Nesterov 2018-07-16 8:47 ` [PATCH v6 6/6] perf probe: Support SDT markers having reference counter (semaphore) Ravi Bangoria 2018-07-16 8:47 ` Ravi Bangoria 2018-07-20 13:47 ` [PATCH v6 0/6] Uprobes: Support SDT markers having reference count (semaphore) Ravi Bangoria 2018-07-20 13:47 ` Ravi Bangoria
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180716084706.28244-6-ravi.bangoria@linux.ibm.com \ --to=ravi.bangoria@linux.ibm.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=alexis.berlemont@gmail.com \ --cc=ananth@linux.vnet.ibm.com \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=linux@armlinux.org.uk \ --cc=mhiramat@kernel.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=naveen.n.rao@linux.vnet.ibm.com \ --cc=oleg@redhat.com \ --cc=paul.burton@mips.com \ --cc=peterz@infradead.org \ --cc=ralf@linux-mips.org \ --cc=rostedt@goodmis.org \ --cc=srikar@linux.vnet.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.