linux-kernel - Re: [RFC PATCH 3/5] bpf: Add helper function for fetching variables at probe point

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: <555A4334.90706@plumgrid.com>
Date:	Mon, 18 May 2015 12:53:24 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	He Kuang <hekuang@...wei.com>, paulus@...ba.org,
	a.p.zijlstra@...llo.nl, mingo@...hat.com, acme@...nel.org,
	namhyung@...nel.org, jolsa@...nel.org, dsahern@...il.com,
	daniel@...earbox.net, brendan.d.gregg@...il.com,
	masami.hiramatsu.pt@...achi.com
CC:	wangnan0@...wei.com, lizefan@...wei.com,
	linux-kernel@...r.kernel.org, pi3orama@....com
Subject: Re: [RFC PATCH 3/5] bpf: Add helper function for fetching variables
 at probe point

On 5/17/15 10:30 PM, He Kuang wrote:
> This helper function uses kernel structure trace_probe and related fetch
> functions for fetching variables described in 'SEC' to bpf stack.
>
> Signed-off-by: He Kuang <hekuang@...wei.com>
...
> +/* Store the value of each argument */
> +static void
> +bpf_store_trace_args(struct pt_regs *regs, struct trace_probe *tp,
> +		u8 *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < tp->nr_args; i++) {
> +		/* Just fetching data normally */
> +		call_fetch(&tp->args[i].fetch, regs,
> +			data + tp->args[i].offset);

that is slower than generating bpf by user space, but more importantly
that's invalid. There is no size check.
r2 in fetch_args points to stack, but nothing checks the stack limits.
You need to add code here to dynamically check it as well.
which will be adding runtime overhead as well.

Your first approach of generating argument accessors in user space
was better.
I think the limit of 3 or 4 arguments was fine.
We need to generate the code for non-debug case anyway,
like my earlier suggestion:
SEC("kprobe/generic_perform_write(void*, void*, long long)")
without debug info it will copy ctx->di into r2, ctx->si into r3
and ctx->dx into r4.

--
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/