| 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: <20150209032253.GG24182@sejong>
Date: Mon, 9 Feb 2015 12:22:53 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Adrian Hunter <adrian.hunter@...el.com>,
linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
Paul Mackerras <paulus@...ba.org>,
Jiri Olsa <jolsa@...nel.org>, Borislav Petkov <bp@...e.de>,
Hemant Kumar <hemant@...ux.vnet.ibm.com>
Subject: Re: [perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE
to remove all caches of FILE
Hi Masami,
On Sat, Feb 07, 2015 at 07:14:19PM +0900, Masami Hiramatsu wrote:
> Add --remove-all FILE to remove all caches of FILE.
> Since the current --remove FILE removes a cache which has
> same build-id of given FILE. Since the command takes a
> FILE path, it can confuse user who tries to remove cache
> about FILE path.
>
> -----
> # ./perf buildid-cache -v --add ./perf
> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> # (update the ./perf binary)
> # ./perf buildid-cache -v --remove ./perf
> Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
> ./perf wasn't in the cache
> -----
> Actually, the --remove's FAIL is not shown, it just silently fails.
>
> So, this patch adds --remove-all FILE action for such usecase.
> perf buildid-cache --remove-all FILE removes all caches which
> has same FILE path.
> In other words, it removes all caches including old binaries.
>
> -----
> # ./perf buildid-cache -v --add ./perf
> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> # (update the ./perf binary)
> # ./perf buildid-cache -v --remove-all ./perf
> Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> -----
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 6 +-
> tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
> tools/perf/util/build-id.c | 81 ++++++++++++++++++-----
> tools/perf/util/build-id.h | 1
> 4 files changed, 115 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 6575dce..01f295e 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -36,7 +36,11 @@ OPTIONS
> actually made.
> -r::
> --remove=::
> - Remove specified file from the cache.
> + Remove a cached binary which has same build-id of specified file
> + from the cache.
> +-R::
> +--remove-all=::
> + Remove all cached binary which has specified path from the cache.
> -M::
> --missing=::
> List missing build ids in the cache for the specified file.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index e7568f5..3a76d51 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
> return err;
> }
>
> +static int build_id_cache__remove_path(const char *pathname)
> +{
> + struct strlist *list;
> + struct str_node *pos;
> + int err;
> +
> + list = build_id_cache__list_build_ids(pathname);
> + if (!list)
> + return 0;
> +
> + strlist__for_each(pos, list) {
> + err = build_id_cache__remove_s(pos->s);
> + if (verbose)
> + pr_info("Removing %s %s: %s\n", pos->s, pathname,
> + err ? "FAIL" : "Ok");
> + if (err)
> + break;
> + }
> + strlist__delete(list);
> +
> + return err;
> +}
> +
> static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
> {
> char filename[PATH_MAX];
> @@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
> bool force = false;
> char const *add_name_list_str = NULL,
> *remove_name_list_str = NULL,
> + *remove_all_name_list_str = NULL,
> *missing_filename = NULL,
> *update_name_list_str = NULL,
> *kcore_filename = NULL;
> @@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
> "file", "kcore file to add"),
> OPT_STRING('r', "remove", &remove_name_list_str, "file list",
> "file(s) to remove"),
> + OPT_STRING('R', "remove-all", &remove_all_name_list_str, "path list",
> + "path(s) to remove (remove old caches too)"),
> OPT_STRING('M', "missing", &missing_filename, "file",
> "to find missing build ids in the cache"),
> OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
> @@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
> }
> }
>
> + if (remove_all_name_list_str) {
> + list = strlist__new(true, remove_all_name_list_str);
> + if (list) {
> + strlist__for_each(pos, list)
> + if (build_id_cache__remove_path(pos->s)) {
> + if (errno == ENOENT) {
> + pr_debug("%s wasn't in the cache\n",
> + pos->s);
> + continue;
> + }
> + pr_warning("Couldn't remove %s: %s\n",
> + pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
> + }
> +
> + strlist__delete(list);
> + }
> + }
> +
> if (missing_filename)
> ret = build_id_cache__fprintf_missing(session, stdout);
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 0bc33be..e688311 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -281,35 +281,84 @@ void disable_buildid_cache(void)
> no_buildid_cache = true;
> }
>
> +static char *build_id_cache__dirname_from_path(const char *name,
> + bool is_kallsyms, bool is_vdso)
> +{
> + char *realname = (char *)name, *filename;
> + bool slash = is_kallsyms || is_vdso;
> +
> + if (!slash) {
> + realname = realpath(name, NULL);
> + if (!realname)
> + return NULL;
> + }
> +
> + if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
> + is_vdso ? DSO__NAME_VDSO : realname) < 0)
> + filename = NULL;
> + free(realname);
Shouldn't it be:
if (!slash)
free(realname);
?
Thanks,
Namhyung
> +
> + return filename;
> +}
> +
> +struct strlist *build_id_cache__list_build_ids(const char *pathname)
> +{
> + struct strlist *list;
> + char *dirname;
> + DIR *dir;
> + struct dirent *d;
> +
> + list = strlist__new(true, NULL);
> + dirname = build_id_cache__dirname_from_path(pathname, false, false);
> + if (!list || !dirname)
> + goto error_free;
> +
> + /* List up all dirents */
> + dir = opendir(dirname);
> + if (!dir)
> + goto error_free;
> + while ((d = readdir(dir)) != NULL) {
> + if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> + continue;
> + strlist__add(list, d->d_name);
> + }
> + closedir(dir);
> +
> + free(dirname);
> + return list;
> +
> +error_free:
> + free(dirname);
> + if (list)
> + strlist__delete(list);
> + return NULL;
> +}
> +
> int build_id_cache__add_s(const char *sbuild_id, const char *name,
> bool is_kallsyms, bool is_vdso)
> {
> const size_t size = PATH_MAX;
> - char *realname, *filename = zalloc(size),
> + char *realname = NULL, *filename = NULL,
> *linkname = zalloc(size), *targetname, *tmp;
> - int len, err = -1;
> - bool slash = is_kallsyms || is_vdso;
> + int err = -1;
>
> - if (is_kallsyms) {
> - if (symbol_conf.kptr_restrict) {
> - pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
> - err = 0;
> - goto out_free;
> - }
> - realname = (char *) name;
> - } else
> + if (!is_kallsyms) {
> realname = realpath(name, NULL);
> + if (!realname)
> + goto out_free;
> + }
>
> - if (realname == NULL || filename == NULL || linkname == NULL)
> + filename = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
> + if (!filename)
> goto out_free;
>
> - len = scnprintf(filename, size, "%s%s%s",
> - buildid_dir, slash ? "/" : "",
> - is_vdso ? DSO__NAME_VDSO : realname);
> if (mkdir_p(filename, 0755))
> goto out_free;
>
> - snprintf(filename + len, size - len, "/%s", sbuild_id);
> + if (asprintf(&filename, "%s/%s", filename, sbuild_id) < 0) {
> + filename = NULL;
> + goto out_free;
> + }
>
> if (access(filename, F_OK)) {
> if (is_kallsyms) {
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 2a09498..cbcadea 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
> int perf_session__write_buildid_table(struct perf_session *session, int fd);
> int perf_session__cache_build_ids(struct perf_session *session);
>
> +struct strlist *build_id_cache__list_build_ids(const char *pathname);
> bool build_id_cache__cached(const char *sbuild_id);
> int build_id_cache__add_s(const char *sbuild_id,
> const char *name, bool is_kallsyms, bool is_vdso);
>
> --
> 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/
--
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