linux-kernel - Re: [PATCH v1] tracing/kprobes: expose maxactive for kretprobe in kprobe

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: <20170328113407.56926eec@gandalf.local.home>
Date:   Tue, 28 Mar 2017 11:34:07 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Alban Crequy <alban.crequy@...il.com>,
        Alban Crequy <alban@...volk.io>,
        Alexei Starovoitov <ast@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Omar Sandoval <osandov@...com>, linux-doc@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        iago@...volk.io, michael@...volk.io,
        "Dorau, Lukasz" <lukasz.dorau@...el.com>
Subject: Re: [PATCH v1] tracing/kprobes: expose maxactive for kretprobe in
 kprobe_events

On Wed, 29 Mar 2017 00:23:35 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:

> > @@ -598,8 +601,10 @@ static int create_trace_kprobe(int argc, char **argv)
> >  {
> >  	/*
> >  	 * Argument syntax:
> > -	 *  - Add kprobe: p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
> > -	 *  - Add kretprobe: r[:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
> > +	 *  - Add kprobe:
> > +	 *      p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
> > +	 *  - Add kretprobe:
> > +	 *      r[MAXACTIVE][:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS]
> >  	 * Fetch args:
> >  	 *  $retval	: fetch return value
> >  	 *  $stack	: fetch stack address
> > @@ -619,6 +624,7 @@ static int create_trace_kprobe(int argc, char **argv)
> >  	int i, ret = 0;
> >  	bool is_return = false, is_delete = false;
> >  	char *symbol = NULL, *event = NULL, *group = NULL;
> > +	int maxactive = 0;
> >  	char *arg;
> >  	unsigned long offset = 0;
> >  	void *addr = NULL;
> > @@ -637,8 +643,26 @@ static int create_trace_kprobe(int argc, char **argv)
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (argv[0][1] == ':') {
> > +	if (is_return && isdigit(argv[0][1]) && strchr(&argv[0][1], ':')) {  
> 
> This only supports r[MAXACTIVE:[GRP/]EVENT]. e.g "r100" without event name.

You mean it doesn't support adding MAXACTIVE without the ':event'.

> 
> Thank you,
> 
> > +		event = strchr(&argv[0][1], ':') + 1;
> > +		event[-1] = '\0';
> > +		ret = kstrtouint(&argv[0][1], 0, &maxactive);
> > +		if (ret) {
> > +			pr_info("Failed to parse maxactive.\n");
> > +			return ret;
> > +		}
> > +		/* kretprobes instances are iterated over via a list. The
> > +		 * maximum should stay reasonable.
> > +		 */
> > +		if (maxactive > 1024) {

Also, can we get rid of magic numbers within the code. There should be a
const or macro defined as MAX_MAXACTIVE or something, and use that here.

-- Steve


> > +			pr_info("Maxactive is too big.\n");
> > +			return -EINVAL;
> > +		}
> > +	} else if (argv[0][1] == ':') {
> >  		event = &argv[0][2];
> > +	}
> > +
> > +	if (event) {
> >  		if (strchr(event, '/')) {
> >  			group = event;
> >  			event = strchr(group, '/') + 1;
> > @@ -718,8 +742,8 @@ static int create_trace_kprobe(int argc, char **argv)
> >  				 is_return ? 'r' : 'p', addr);
> >  		event = buf;
> >  	}
> > -	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, argc,
> > -			       is_return);
> > +	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
> > +			       argc, is_return);
> >  	if (IS_ERR(tk)) {
> >  		pr_info("Failed to allocate trace_probe.(%d)\n",
> >  			(int)PTR_ERR(tk));
> > -- 
> > 2.7.4
> >   
> 
>