| 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
| ||
|
Date: Tue, 12 Dec 2017 23:46:00 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: bhargavb <bhargavaramudu@...il.com>, linux-kernel@...r.kernel.org,
Paul Clarke <pc@...ibm.com>,
Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>,
Thomas Richter <tmricht@...ux.vnet.ibm.com>,
linux-rt-users@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v3 5/5] perf-probe: Support escaped character in parser
On Mon, 11 Dec 2017 17:03:30 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> Em Sat, Dec 09, 2017 at 01:28:41AM +0900, Masami Hiramatsu escreveu:
> > Support the special characters escaped by '\' in parser.
> > This allows user to specify versions directly like below.
> >
> > =====
> > # ./perf probe -x /lib64/libc-2.25.so malloc_get_state\\@GLIBC_2.2.5
> > Added new event:
> > probe_libc:malloc_get_state (on malloc_get_state@...BC_2.2.5 in /usr/lib64/libc-2.25.so)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe_libc:malloc_get_state -aR sleep 1
> >
> > =====
> >
> > Or, you can use separators in source filename, e.g.
> >
> > =====
> > # ./perf probe -x /opt/test/a.out foo+bar.c:3
> > Semantic error :There is non-digit character in offset.
> > Error: Command Parse Error.
> > =====
> >
> > Usually "+" in source file cause parser error, but
> >
> > =====
> > # ./perf probe -x /opt/test/a.out foo\\+bar.c:4
> > Added new event:
> > probe_a:main (on @foo+bar.c:4 in /opt/test/a.out)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e probe_a:main -aR sleep 1
> > =====
> >
> > escaped "\+" allows you to specify that.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> > Reviewed-by: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> > Acked-by: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
> > ---
> > Changes in v2:
> > - Add a description and samples in Documentation/perf-probe.txt
> > Changes in v3:
> > - Update for new series.
> > ---
> > tools/perf/Documentation/perf-probe.txt | 16 +++++++++
> > tools/perf/util/probe-event.c | 57 ++++++++++++++++++-------------
> > tools/perf/util/string.c | 46 +++++++++++++++++++++++++
> > tools/perf/util/string2.h | 2 +
> > 4 files changed, 98 insertions(+), 23 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> > index f96382692f42..b6866a05edd2 100644
> > --- a/tools/perf/Documentation/perf-probe.txt
> > +++ b/tools/perf/Documentation/perf-probe.txt
> > @@ -182,6 +182,14 @@ Note that before using the SDT event, the target binary (on which SDT events are
> > For details of the SDT, see below.
> > https://sourceware.org/gdb/onlinedocs/gdb/Static-Probe-Points.html
> >
> > +ESCAPED CHARACTER
> > +-----------------
> > +
> > +In the probe syntax, '=', '@', '+', ':' and ';' are treated as a special character. You can use a backslash ('\') to escape the special characters.
> > +This is useful if you need to probe on a specific versioned symbols, like @GLIBC_... suffixes, or also you need to specify a source file which includes the special characters.
> > +Note that usually single backslash is consumed by shell, so you might need to pass double backslash (\\) or wrapping with single quotes (\'AAA\@BBB').
> > +See EXAMPLES how it is used.
> > +
> > PROBE ARGUMENT
> > --------------
> > Each probe argument follows below syntax.
> > @@ -277,6 +285,14 @@ Add a USDT probe to a target process running in a different mount namespace
> >
> > ./perf probe --target-ns <target pid> -x /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.121-0.b13.el7_3.x86_64/jre/lib/amd64/server/libjvm.so %sdt_hotspot:thread__sleep__end
> >
> > +Add a probe on specific versioned symbol by backslash escape
> > +
> > + ./perf probe -x /lib64/libc-2.25.so 'malloc_get_state\@GLIBC_2.2.5'
> > +
> > +Add a probe in a source file using special characters by backslash escape
> > +
> > + ./perf probe -x /opt/test/a.out 'foo\+bar.c:4'
> > +
> >
> > SEE ALSO
> > --------
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 0d6c66d51939..735972e7bc6b 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1325,27 +1325,30 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
> > {
> > char *ptr;
> >
> > - ptr = strchr(*arg, ':');
> > + ptr = strpbrk_esc(*arg, ":");
> > if (ptr) {
> > *ptr = '\0';
> > if (!pev->sdt && !is_c_func_name(*arg))
> > goto ng_name;
> > - pev->group = strdup(*arg);
> > + pev->group = strdup_esc(*arg);
> > if (!pev->group)
> > return -ENOMEM;
> > *arg = ptr + 1;
> > } else
> > pev->group = NULL;
> > - if (!pev->sdt && !is_c_func_name(*arg)) {
> > +
> > + pev->event = strdup_esc(*arg);
> > + if (pev->event == NULL)
> > + return -ENOMEM;
> > +
> > + if (!pev->sdt && !is_c_func_name(pev->event)) {
> > + zfree(&pev->event);
> > ng_name:
> > + zfree(&pev->group);
> > semantic_error("%s is bad for event name -it must "
> > "follow C symbol-naming rule.\n", *arg);
> > return -EINVAL;
> > }
> > - pev->event = strdup(*arg);
> > - if (pev->event == NULL)
> > - return -ENOMEM;
> > -
> > return 0;
> > }
> >
> > @@ -1373,7 +1376,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > arg++;
> > }
> >
> > - ptr = strpbrk(arg, ";=@+%");
> > + ptr = strpbrk_esc(arg, ";=@+%");
> > if (pev->sdt) {
> > if (ptr) {
> > if (*ptr != '@') {
> > @@ -1387,7 +1390,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > pev->target = build_id_cache__origname(tmp);
> > free(tmp);
> > } else
> > - pev->target = strdup(ptr + 1);
> > + pev->target = strdup_esc(ptr + 1);
> > if (!pev->target)
> > return -ENOMEM;
> > *ptr = '\0';
> > @@ -1421,13 +1424,14 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > *
> > * Otherwise, we consider arg to be a function specification.
> > */
> > - if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> > + if (!strpbrk_esc(arg, "+@%")) {
> > + ptr = strpbrk_esc(arg, ";:");
> > /* This is a file spec if it includes a '.' before ; or : */
> > - if (memchr(arg, '.', ptr - arg))
> > + if (ptr && memchr(arg, '.', ptr - arg))
> > file_spec = true;
> > }
> >
> > - ptr = strpbrk(arg, ";:+@%");
> > + ptr = strpbrk_esc(arg, ";:+@%");
> > if (ptr) {
> > nc = *ptr;
> > *ptr++ = '\0';
> > @@ -1436,7 +1440,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > if (arg[0] == '\0')
> > tmp = NULL;
> > else {
> > - tmp = strdup(arg);
> > + tmp = strdup_esc(arg);
> > if (tmp == NULL)
> > return -ENOMEM;
> > }
> > @@ -1469,12 +1473,12 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > arg = ptr;
> > c = nc;
> > if (c == ';') { /* Lazy pattern must be the last part */
> > - pp->lazy_line = strdup(arg);
> > + pp->lazy_line = strdup(arg); /* let leave escapes */
> > if (pp->lazy_line == NULL)
> > return -ENOMEM;
> > break;
> > }
> > - ptr = strpbrk(arg, ";:+@%");
> > + ptr = strpbrk_esc(arg, ";:+@%");
> > if (ptr) {
> > nc = *ptr;
> > *ptr++ = '\0';
> > @@ -1501,7 +1505,7 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > semantic_error("SRC@SRC is not allowed.\n");
> > return -EINVAL;
> > }
> > - pp->file = strdup(arg);
> > + pp->file = strdup_esc(arg);
> > if (pp->file == NULL)
> > return -ENOMEM;
> > break;
> > @@ -2803,23 +2807,30 @@ static int find_probe_functions(struct map *map, char *name,
> > struct rb_node *tmp;
> > const char *norm, *ver;
> > char *buf = NULL;
> > + bool cut_version = true;
> >
> > if (map__load(map) < 0)
> > return 0;
> >
> > + /* If user gives a version, don't cut off the version from symbols */
> > + if (strchr(name, '@'))
> > + cut_version = false;
> > +
> > map__for_each_symbol(map, sym, tmp) {
> > norm = arch__normalize_symbol_name(sym->name);
> > if (!norm)
> > continue;
> >
> > - /* We don't care about default symbol or not */
> > - ver = strchr(norm, '@');
> > - if (ver) {
> > - buf = strndup(norm, ver - norm);
> > - if (!buf)
> > - return -ENOMEM;
> > - norm = buf;
> > + if (cut_version) {
> > + /* We don't care about default symbol or not */
> > + ver = strchr(norm, '@');
> > + if (ver) {
> > + buf = strndup(norm, ver - norm);
> > + if (!buf)
> > + return -ENOMEM;
> > + norm = buf;
>
> You forgot a } here, please check this logic and resubmit just this last
> patch, without the string.c and string2.h part, that I already split
> from this one and applied.
OOPS! thanks! I missed something around that.... maybe while updating patches.
Hmm, I might be so upset...
OK, anyway, I'll do that.
>
> gcc diagnostics:
>
> LD /tmp/build/perf/perf-in.o
> util/probe-event.c: In function ‘find_probe_functions’:
> util/probe-event.c:2848:17: error: declaration of ‘map’ shadows a parameter [-Werror=shadow]
> struct map *map __maybe_unused,
> ^~~
> util/probe-event.c:2802:45: note: shadowed declaration is here
> static int find_probe_functions(struct map *map, char *name,
> ^~~
> util/probe-event.c:2849:20: error: declaration of ‘sym’ shadows a previous local [-Werror=shadow]
> struct symbol *sym __maybe_unused) { }
>
> ------------------------
>
> clang's:
>
> CC /tmp/build/perf/util/probe-event.o
> util/probe-event.c:2849:40: error: function definition is not allowed here
> struct symbol *sym __maybe_unused) { }
> ^
> util/probe-event.c:2857:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3009:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3098:1: error: function definition is not allowed here
> {
> ^
> util/probe-event.c:3112:1: error: function definition is not allowed here
>
> ------------------------
>
> Neither are that informative... ;-\
Sorry, that's my mistake...
Thank you,
>
> - Arnaldo
>
> > }
> > +
> > if (strglobmatch(norm, name)) {
> > found++;
> > if (syms && found < probe_conf.max_probes)
> > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > index aaa08ee8c717..d8bfd0c4d2cb 100644
> > --- a/tools/perf/util/string.c
> > +++ b/tools/perf/util/string.c
> > @@ -396,3 +396,49 @@ char *asprintf_expr_inout_ints(const char *var, bool in, size_t nints, int *ints
> > free(expr);
> > return NULL;
> > }
> > +
> > +/* Like strpbrk(), but not break if it is right after a backslash (escaped) */
> > +char *strpbrk_esc(char *str, const char *stopset)
> > +{
> > + char *ptr;
> > +
> > + do {
> > + ptr = strpbrk(str, stopset);
> > + if (ptr == str ||
> > + (ptr == str + 1 && *(ptr - 1) != '\\'))
> > + break;
> > + str = ptr + 1;
> > + } while (ptr && *(ptr - 1) == '\\' && *(ptr - 2) != '\\');
> > +
> > + return ptr;
> > +}
> > +
> > +/* Like strdup, but do not copy a single backslash */
> > +char *strdup_esc(const char *str)
> > +{
> > + char *s, *d, *p, *ret = strdup(str);
> > +
> > + if (!ret)
> > + return NULL;
> > +
> > + d = strchr(ret, '\\');
> > + if (!d)
> > + return ret;
> > +
> > + s = d + 1;
> > + do {
> > + if (*s == '\0') {
> > + *d = '\0';
> > + break;
> > + }
> > + p = strchr(s + 1, '\\');
> > + if (p) {
> > + memmove(d, s, p - s);
> > + d += p - s;
> > + s = p + 1;
> > + } else
> > + memmove(d, s, strlen(s) + 1);
> > + } while (p);
> > +
> > + return ret;
> > +}
> > diff --git a/tools/perf/util/string2.h b/tools/perf/util/string2.h
> > index ee14ca5451ab..4c68a09b97e8 100644
> > --- a/tools/perf/util/string2.h
> > +++ b/tools/perf/util/string2.h
> > @@ -39,5 +39,7 @@ static inline char *asprintf_expr_not_in_ints(const char *var, size_t nints, int
> > return asprintf_expr_inout_ints(var, false, nints, ints);
> > }
> >
> > +char *strpbrk_esc(char *str, const char *stopset);
> > +char *strdup_esc(const char *str);
> >
> > #endif /* PERF_STRING_H */
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists