Re: [PATCH] uprobe: Warn if unable to install breakpoint

On 2017/09/01 03:09PM, Michael Ellerman wrote:

"Naveen N. Rao" [off-list ref] writes:

quoted

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 <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
 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 0e137f98a50c..587c591a535c 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);

That should probably be ratelimited no?

Uprobes can only be installed by root today, so it is not as bad. But, I 
do agree that it is good to ratelimit. I will send a subsequent patch to 
do this.

Thanks for the review,
- Naveen