Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1731839 > unrolled thread

[PATCH v2 1/3] kernel/uprobes: Warn if unable to install breakpoint

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/3] kernel/uprobes: Warn if unable to install breakpoint "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-09-13 22:30 +0200
    Re: [PATCH v2 1/3] kernel/uprobes: Warn if unable to install  breakpoint Oleg Nesterov <oleg@redhat.com> - 2017-09-15 18:00 +0200
      Re: [PATCH v2 1/3] kernel/uprobes: Warn if unable to install  breakpoint "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-09-16 13:50 +0200

#1731839 — [PATCH v2 1/3] kernel/uprobes: Warn if unable to install breakpoint

When we try to install a uprobe breakpoint in uprobe_mmap(), we ignore
all errors encountered in the process per this comment at the top of
the function:
    /*
     * Called from mmap_region/vma_adjust with mm->mmap_sem acquired.
     *
     * Currently we ignore all errors and always return 0, the callers
     * can't handle the failure anyway.
     */

However, this is very confusing for users since no probe hits are
recorded nor is an error logged in dmesg.

Fix this by logging an error in dmesg so that users can discover that
there was an issue with the uprobe. To facilitate use of uprobe_warn(),
we move that function to the top of the file.

With this patch, we see a message similar to this in dmesg:
    [  201.449213] uprobe: uprobe_t:9740 failed to setup probe at 0x95c (-524)

Reported-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/events/uprobes.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 267f6ef91d97..4af1acff9cc3 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -112,6 +112,12 @@ struct xol_area {
 	unsigned long 			vaddr;		/* Page(s) of instruction slots */
 };
 
+static void uprobe_warn(struct task_struct *t, const char *msg)
+{
+	pr_warn("uprobe: %s:%d failed to %s\n",
+			current->comm, current->pid, msg);
+}
+
 /*
  * valid_vma: Verify if the specified vma is an executable vma
  * Relax restrictions while unregistering: vm_flags might have
@@ -1087,7 +1093,14 @@ int uprobe_mmap(struct vm_area_struct *vma)
 		if (!fatal_signal_pending(current) &&
 		    filter_chain(uprobe, UPROBE_FILTER_MMAP, vma->vm_mm)) {
 			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
-			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
+			int ret = install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
+			if (ret) {
+				char msg[64];
+				snprintf(msg, sizeof(msg),
+						"setup probe at 0x%llx (%d)",
+						uprobe->offset, ret);
+				uprobe_warn(current, (const char *)msg);
+			}
 		}
 		put_uprobe(uprobe);
 	}
@@ -1468,12 +1481,6 @@ static int dup_utask(struct task_struct *t, struct uprobe_task *o_utask)
 	return 0;
 }
 
-static void uprobe_warn(struct task_struct *t, const char *msg)
-{
-	pr_warn("uprobe: %s:%d failed to %s\n",
-			current->comm, current->pid, msg);
-}
-
 static void dup_xol_work(struct callback_head *work)
 {
 	if (current->flags & PF_EXITING)
-- 
2.14.1

[toc] | [next] | [standalone]

#1732933 — Re: [PATCH v2 1/3] kernel/uprobes: Warn if unable to install breakpoint

On 09/14, Naveen N. Rao wrote:
>
> +static void uprobe_warn(struct task_struct *t, const char *msg)
> +{
> +	pr_warn("uprobe: %s:%d failed to %s\n",
> +			current->comm, current->pid, msg);
> +}
> +
>  /*
>   * valid_vma: Verify if the specified vma is an executable vma
>   * Relax restrictions while unregistering: vm_flags might have
> @@ -1087,7 +1093,14 @@ int uprobe_mmap(struct vm_area_struct *vma)
>  		if (!fatal_signal_pending(current) &&
>  		    filter_chain(uprobe, UPROBE_FILTER_MMAP, vma->vm_mm)) {
>  			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
> -			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
> +			int ret = install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
> +			if (ret) {
> +				char msg[64];
> +				snprintf(msg, sizeof(msg),
> +						"setup probe at 0x%llx (%d)",
> +						uprobe->offset, ret);
> +				uprobe_warn(current, (const char *)msg);

Agreed, but... this is cosmetic, but I don't really like this snprintf().

I won't insist too much, but wouldn't it better to turn uprobe_warn() into
uprobe_warn(struct task_struct *t, char *fmt, ...) ?

Oleg.

[toc] | [prev] | [next] | [standalone]

#1733263 — Re: [PATCH v2 1/3] kernel/uprobes: Warn if unable to install breakpoint

On 2017/09/15 05:53PM, Oleg Nesterov wrote:
> On 09/14, Naveen N. Rao wrote:
> >
> > +static void uprobe_warn(struct task_struct *t, const char *msg)
> > +{
> > +	pr_warn("uprobe: %s:%d failed to %s\n",
> > +			current->comm, current->pid, msg);
> > +}
> > +
> >  /*
> >   * valid_vma: Verify if the specified vma is an executable vma
> >   * Relax restrictions while unregistering: vm_flags might have
> > @@ -1087,7 +1093,14 @@ int uprobe_mmap(struct vm_area_struct *vma)
> >  		if (!fatal_signal_pending(current) &&
> >  		    filter_chain(uprobe, UPROBE_FILTER_MMAP, vma->vm_mm)) {
> >  			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
> > -			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
> > +			int ret = install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
> > +			if (ret) {
> > +				char msg[64];
> > +				snprintf(msg, sizeof(msg),
> > +						"setup probe at 0x%llx (%d)",
> > +						uprobe->offset, ret);
> > +				uprobe_warn(current, (const char *)msg);
> 
> Agreed, but... this is cosmetic, but I don't really like this snprintf().
> 
> I won't insist too much, but wouldn't it better to turn uprobe_warn() into
> uprobe_warn(struct task_struct *t, char *fmt, ...) ?

Good point. In fact, I think we can just use pr_fmt().

Thanks,
Naveen

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web