linux-kernel - [PATCH v3 1/3] kernel/uprobes: Do away with uprobe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <d43e3443fda0e788d02e1f621294ce4a572a644f.1506061547.git.naveen.n.rao@linux.vnet.ibm.com>
Date:   Fri, 22 Sep 2017 14:43:36 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Oleg Nesterov <oleg@...hat.com>,
        Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Anton Blanchard <anton@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        linux-kernel@...r.kernel.org
Subject: [PATCH v3 1/3] kernel/uprobes: Do away with uprobe_warn()

uprobe_warn() was added in commit 248d3a7b2f1000 ("uprobes: Change
uprobe_copy_process() to dup return_instances") to have a uniform format
for warnings thrown from the uprobes subsystem. Though it has served
its role, there are a couple of issues with it:
1. Though it was meant to accept a 'struct task_struct *' and use that,
the actual code just uses 'current', completely ignoring the task
structure being passed in.
2. It does not accept varargs currently.
3. The strings are split making it difficult to grep for failures.

To address these and simplify things, this patch moves to using pr_fmt()
to encode the uprobe string prefix for all printk's in this file. The
prefix string uses 'current->comm' and 'current->pid', so we update the
messages in uprobe_copy_process() to explicitly refer to the other task
structure.

Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
---
 kernel/events/uprobes.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 267f6ef91d97..18bd6127d00e 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -42,6 +42,11 @@
 
 #include <linux/uprobes.h>
 
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+#define pr_fmt(fmt) "uprobe: %s:%d " fmt, current->comm, current->pid
+
 #define UINSNS_PER_PAGE			(PAGE_SIZE/UPROBE_XOL_SLOT_BYTES)
 #define MAX_UPROBE_XOL_SLOTS		UINSNS_PER_PAGE
 
@@ -1468,12 +1473,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)
@@ -1481,7 +1480,7 @@ static void dup_xol_work(struct callback_head *work)
 
 	if (!__create_xol_area(current->utask->dup_xol_addr) &&
 			!fatal_signal_pending(current))
-		uprobe_warn(current, "dup xol area");
+		pr_warn("failed to dup xol area");
 }
 
 /*
@@ -1501,13 +1500,17 @@ void uprobe_copy_process(struct task_struct *t, unsigned long flags)
 	if (mm == t->mm && !(flags & CLONE_VFORK))
 		return;
 
-	if (dup_utask(t, utask))
-		return uprobe_warn(t, "dup ret instances");
+	if (dup_utask(t, utask)) {
+		pr_warn("failed to dup ret instances for %s:%d", t->comm, t->pid);
+		return;
+	}
 
 	/* The task can fork() after dup_xol_work() fails */
 	area = mm->uprobes_state.xol_area;
-	if (!area)
-		return uprobe_warn(t, "dup xol area");
+	if (!area) {
+		pr_warn("failed to dup xol area for %s:%d", t->comm, t->pid);
+		return;
+	}
 
 	if (mm == t->mm)
 		return;
@@ -1594,7 +1597,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
 			 * This situation is not possible. Likely we have an
 			 * attack from user-space.
 			 */
-			uprobe_warn(current, "handle tail call");
+			pr_warn("failed to handle tail call");
 			goto fail;
 		}
 		orig_ret_vaddr = utask->return_instances->orig_ret_vaddr;
@@ -1853,7 +1856,7 @@ static void handle_trampoline(struct pt_regs *regs)
 	return;
 
  sigill:
-	uprobe_warn(current, "handle uretprobe, sending SIGILL.");
+	pr_warn("failed to handle uretprobe, sending SIGILL.");
 	force_sig_info(SIGILL, SEND_SIG_FORCED, current);
 
 }
@@ -1961,7 +1964,7 @@ static void handle_singlestep(struct uprobe_task *utask, struct pt_regs *regs)
 	spin_unlock_irq(&current->sighand->siglock);
 
 	if (unlikely(err)) {
-		uprobe_warn(current, "execute the probed insn, sending SIGILL.");
+		pr_warn("failed to execute the probed insn, sending SIGILL.");
 		force_sig_info(SIGILL, SEND_SIG_FORCED, current);
 	}
 }
-- 
2.14.1