| 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
| ||
|
Message-ID: <20120629184937.GD7847@infradead.org>
Date: Fri, 29 Jun 2012 15:49:37 -0300
From: Arnaldo Carvalho de Melo <acme@...hat.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: a.p.zijlstra@...llo.nl, mingo@...e.hu, paulus@...ba.org,
cjashfor@...ux.vnet.ibm.com, fweisbec@...il.com,
eranian@...gle.com, gorcunov@...nvz.org, tzanussi@...il.com,
mhiramat@...hat.com, robert.richter@....com, fche@...hat.com,
linux-kernel@...r.kernel.org, masami.hiramatsu.pt@...achi.com,
drepper@...il.com, asharma@...com, benjamin.redelings@...cent.org
Subject: Re: [PATCH 12/19] perf, tool: Back [vdso] DSO with real data
Em Mon, Jun 11, 2012 at 03:20:07PM +0200, Jiri Olsa escreveu:
> Storing data for VDSO shared object, because we need it for
> the unwind process.
>
> The idea is that VDSO shared object is same for all process
> on a running system, so it makes no difference if we store
> it inside the tracer - perf.
>
> The record command:
> When [vdso] map memory is hit, we retrieve [vdso] DSO image
> and store it into temporary file. During the build-id
> processing the [vdso] DSO image is stored in build-id db,
> and build-id refference is made inside perf.data. The temporary
> file is removed when record is finished.
>
> The report command:
> We read build-id from perf.data and store [vdso] DSO object.
> This object is refferenced and attached to map when the MMAP
> events are processed. Thus during the SAMPLE event processing
> we have correct mmap/dso attached.
>
> Adding following API for vdso object:
> vdso__file
> - vdso temp file path
>
> vdso__get_file
> - finds and store VDSO image into temp file,
> the temp file path is returned
>
> vdso__exit
> - removes temporary VDSO image if there's any
>
> Signed-off-by: Jiri Olsa <jolsa@...hat.com>
> ---
> tools/perf/Makefile | 2 +
> tools/perf/util/map.c | 23 ++++++++++-
> tools/perf/util/session.c | 2 +
> tools/perf/util/vdso.c | 90 +++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/vdso.h | 8 ++++
> 5 files changed, 123 insertions(+), 2 deletions(-)
> create mode 100644 tools/perf/util/vdso.c
> create mode 100644 tools/perf/util/vdso.h
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 0eee64c..e48b969 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -319,6 +319,7 @@ LIB_H += $(ARCH_INCLUDE)
> LIB_H += util/cgroup.h
> LIB_H += $(TRACE_EVENT_DIR)event-parse.h
> LIB_H += util/target.h
> +LIB_H += util/vdso.h
>
> LIB_OBJS += $(OUTPUT)util/abspath.o
> LIB_OBJS += $(OUTPUT)util/alias.o
> @@ -382,6 +383,7 @@ LIB_OBJS += $(OUTPUT)util/xyarray.o
> LIB_OBJS += $(OUTPUT)util/cpumap.o
> LIB_OBJS += $(OUTPUT)util/cgroup.o
> LIB_OBJS += $(OUTPUT)util/target.o
> +LIB_OBJS += $(OUTPUT)util/vdso.o
>
> BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
>
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 35ae568..1649ea0 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -7,6 +7,7 @@
> #include <stdio.h>
> #include <unistd.h>
> #include "map.h"
> +#include "vdso.h"
>
> const char *map_type__name[MAP__NR_TYPES] = {
> [MAP__FUNCTION] = "Functions",
> @@ -18,10 +19,14 @@ static inline int is_anon_memory(const char *filename)
> return strcmp(filename, "//anon") == 0;
> }
>
> +static inline int is_vdso_memory(const char *filename)
> +{
> + return !strcmp(filename, "[vdso]");
> +}
> +
> static inline int is_no_dso_memory(const char *filename)
> {
> return !strcmp(filename, "[stack]") ||
> - !strcmp(filename, "[vdso]") ||
> !strcmp(filename, "[heap]");
> }
>
> @@ -50,9 +55,10 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
> if (self != NULL) {
> char newfilename[PATH_MAX];
> struct dso *dso;
> - int anon, no_dso;
> + int anon, no_dso, vdso;
>
> anon = is_anon_memory(filename);
> + vdso = is_vdso_memory(filename);
> no_dso = is_no_dso_memory(filename);
>
> if (anon) {
> @@ -60,10 +66,23 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
> filename = newfilename;
> }
>
> + if (vdso) {
> + filename = (char *) vdso__file;
> + pgoff = 0;
> + }
> +
> dso = __dsos__findnew(dsos__list, filename);
> if (dso == NULL)
> goto out_delete;
>
> + if (vdso && !dso->has_build_id) {
> + char *file_vdso = vdso__get_file();
> + if (file_vdso)
> + dso__set_long_name(dso, file_vdso);
> + else
> + no_dso = 1;
> + }
> +
> map__init(self, type, start, start + len, pgoff, dso);
>
> if (anon || no_dso) {
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 2785ce8..f400612 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -14,6 +14,7 @@
> #include "sort.h"
> #include "util.h"
> #include "cpumap.h"
> +#include "vdso.h"
>
> static int perf_session__open(struct perf_session *self, bool force)
> {
> @@ -209,6 +210,7 @@ void perf_session__delete(struct perf_session *self)
> machine__exit(&self->host_machine);
> close(self->fd);
> free(self);
> + vdso__exit();
> }
>
> void machine__remove_thread(struct machine *self, struct thread *th)
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> new file mode 100644
> index 0000000..e964482
> --- /dev/null
> +++ b/tools/perf/util/vdso.c
> @@ -0,0 +1,90 @@
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <linux/kernel.h>
> +#include "vdso.h"
> +#include "util.h"
> +
> +const char vdso__file[] = "/tmp/vdso.so";
Oops, can you please use mkstemp()?
> +static bool vdso_found;
> +
> +static int find_vdso_map(void **start, void **end)
> +{
> + FILE *maps;
> + char line[128];
> + int found = 0;
> +
> + maps = fopen("/proc/self/maps", "r");
> + if (!maps) {
> + pr_err("vdso: cannot open maps\n");
> + return -1;
> + }
> +
> + while (!found && fgets(line, sizeof(line), maps)) {
> + int m = -1;
> +
> + /* We care only about private r-x mappings. */
> + if (2 != sscanf(line, "%p-%p r-xp %*x %*x:%*x %*u %n",
> + start, end, &m))
> + continue;
> + if (m < 0)
> + continue;
> +
> + if (!strncmp(&line[m], "[vdso]", 6))
> + found = 1;
> + }
> +
> + fclose(maps);
> + return !found;
> +}
> +
> +char *vdso__get_file(void)
> +{
> + char *vdso = NULL;
> + char *buf = NULL;
> + void *start, *end;
> +
> + do {
> + int fd, size;
> +
> + if (vdso_found) {
> + vdso = (char *) vdso__file;
codying stile bzzt, use:
vdso = (char *)vdso__file;
> + break;
> + }
> +
> + if (find_vdso_map(&start, &end))
> + break;
> +
> + size = end - start;
> + buf = malloc(size);
> + if (!buf)
> + break;
> +
> + memcpy(buf, start, size);
Introduce memdup, just like in the kernel we have kmemdup?
> +
> + fd = open(vdso__file, O_CREAT|O_WRONLY|O_TRUNC, S_IRWXU);
> + if (fd < 0)
> + break;
> +
> + if (size == write(fd, buf, size))
> + vdso = (char *) vdso__file;
coding style
> +
> + close(fd);
> + } while (0);
And what is the point of this while construct?
> +
> + if (buf)
> + free(buf);
No need to check, free accepts NULL pointers.
> +
> + vdso_found = (vdso != NULL);
> + return vdso;
> +}
> +
> +void vdso__exit(void)
> +{
> + if (vdso_found)
> + unlink(vdso__file);
What is the point of using a global variable to check if you need to
call unlink if you don't check unlink's result? Just ditch vdso_found
and call unlink unconditionally, but on the temp file created.
And be careful how you use that temp filename...
> +}
> diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
> new file mode 100644
> index 0000000..908b041
> --- /dev/null
> +++ b/tools/perf/util/vdso.h
> @@ -0,0 +1,8 @@
> +#ifndef __VDSO__
> +#define __VDSO__
Better use __PERF_VDSO__
> +
> +extern const char vdso__file[];
> +char *vdso__get_file(void);
> +void vdso__exit(void);
> +
> +#endif /* __VDSO__ */
> --
> 1.7.7.6
--
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/
Powered by blists - more mailing lists