linux-kernel - [PATCH] uprobes: Allocate ->utask before handler

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] [day] [month] [year] [list]

Message-ID: <20131125141202.GA28937@redhat.com>
Date:	Mon, 25 Nov 2013 15:12:02 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Namhyung Kim <namhyung@...nel.org>
Cc:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Namhyung Kim <namhyung.kim@....com>,
	Hyeoncheol Lee <cheol.lee@....com>,
	Hemant Kumar <hkshaw@...ux.vnet.ibm.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	"zhangwei(Jovi)" <jovi.zhangwei@...wei.com>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Subject: [PATCH] uprobes: Allocate ->utask before handler_chain() for
	tracing handlers

Hi Namhyung,

On 11/25, Namhyung Kim wrote:
>
> Hi Oleg,
>
> On Tue, 12 Nov 2013 17:00:01 +0900, Namhyung Kim wrote:
> > For @+addr syntax: user-space uses relative symbol address from a loaded
> >                    base address and kernel calculates the base address
> >                    using "current->utask->vaddr - tu->offset".
>
> I tried this approach and realized that current->utask is not set

Aaah, I am stipid. Yes, I forgot that it is always NULL until the
task does xol or prepare_uretprobe() for the 1st time.... And on
powerpc it can be always NULL because it can likely emulate the
probed insn.

> or has
> an invalid vaddr when handler_chain() is called.

But this should not matter at all? you should not rely on the value of
->vaddr, you should use at as uninitialized scratchpad. And in fact we
were going to add another member into the union which should be used
instead later (but lets ignore this for now).

> So I had to apply
> following patch and it seems to work well for me.  Could you confirm it?

I don't think we need this patch, see below.

> @@ -1744,11 +1744,17 @@ static void handle_swbp(struct pt_regs *regs)
>  	if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags)))
>  		goto out;
>
> +	utask = get_utask();

Yes, we need this until we find another way to pass the additional info
to ->fetch() methods. This is a bit unfortunate, may be we fill find a
better solution later.

But until then we can probably tolerate the hack below, what do you
think?

Oleg.
---

Subject: [PATCH] uprobes: Allocate ->utask before handler_chain() for tracing handlers

uprobe_trace_print() and uprobe_perf_print() need to pass the additional
info to call_fetch() methods, currently there is no simple way to do this.

current->utask looks like a natural place to hold this info, but we need
to allocate it before handler_chain().

This is a bit unfortunate, perhaps we will find a better solution later,
but this is simple and should work right now.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---
 kernel/events/uprobes.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index b886a5e..307d87c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1854,6 +1854,10 @@ static void handle_swbp(struct pt_regs *regs)
 	if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags)))
 		goto out;
 
+	/* Tracing handlers use ->utask to communicate with fetch methods */
+	if (!get_utask())
+		goto out;
+
 	handler_chain(uprobe, regs);
 	if (can_skip_sstep(uprobe, regs))
 		goto out;
-- 
1.5.5.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/