lkml.org 
[lkml]   [2015]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/2] perf probe: Only call probe_file__get_events() when fd is valid
    Em Thu, Nov 05, 2015 at 03:07:23PM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
    > From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
    > >Em Thu, Nov 05, 2015 at 01:19:24PM +0000, Wang Nan escreveu:
    > >> In system with kprobe enabled but uprobe turned off, 'perf probe -d'
    > >> causes segfault because it calls probe_file__get_events() with a
    > >> negative fd (when deleting uprobe events).
    > >>
    > >> This patch validates fds before calling probe_file__get_events().
    > >
    > >Wouldn't this shorter patch be more robust by deferring the validation
    > >to just before using the 'fd' value?
    > >
    > >The end result is that probe_file__get_events() will return -ENOENT in
    > >both calls, so ret and ret2 will both be set to -ENOENT, as in your
    > >patch.
    > >
    > >Masami?
    >
    > Yes, I've suggested so :)

    Kinda, you suggested to check at probe_file__get_events() , while I am
    suggesting to check even deeper, just before the actual use of fd, in
    probe_file__get_rawlist().

    - Arnaldo

    > https://lkml.org/lkml/2015/11/5/353
    >
    > Thanks!
    >
    >
    >
    > >
    > >- Arnaldo
    > >
    > >diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
    > >index 89dbeb92c68e..f04a8318a1a7 100644
    > >--- a/tools/perf/util/probe-file.c
    > >+++ b/tools/perf/util/probe-file.c
    > >@@ -138,6 +138,9 @@ struct strlist *probe_file__get_rawlist(int fd)
    > > char *p;
    > > struct strlist *sl;
    > >
    > >+ if (fd < 0)
    > >+ return NULL;
    > >+
    > > sl = strlist__new(NULL, NULL);
    > >
    > > fp = fdopen(dup(fd), "r");
    > >
    > >
    > >diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
    > >index 89dbeb92c68e..e5dc8e62f0f1 100644
    > >--- a/tools/perf/util/probe-file.c
    > >+++ b/tools/perf/util/probe-file.c
    > >@@ -169,6 +169,9 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
    > > struct probe_trace_event tev;
    > > int ret = 0;
    > >
    > >+ if (fd < 0)
    > >+ return NULL;
    > >+
    > > memset(&tev, 0, sizeof(tev));
    > > rawlist = probe_file__get_rawlist(fd);
    > > if (!rawlist)
    > >
    > >
    > >> Signed-off-by: Wang Nan <wangnan0@huawei.com>
    > >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
    > >> Cc: Jiri Olsa <jolsa@kernel.org>
    > >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    > >> Cc: Namhyung Kim <namhyung@kernel.org>
    > >> ---
    > >> tools/perf/builtin-probe.c | 12 ++++++++++--
    > >> 1 file changed, 10 insertions(+), 2 deletions(-)
    > >>
    > >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
    > >> index 132afc9..861aa89 100644
    > >> --- a/tools/perf/builtin-probe.c
    > >> +++ b/tools/perf/builtin-probe.c
    > >> @@ -384,7 +384,11 @@ static int perf_del_probe_events(struct strfilter *filter)
    > >> goto out;
    > >> }
    > >>
    > >> - ret = probe_file__get_events(kfd, filter, klist);
    > >> + if (kfd < 0)
    > >> + ret = -ENOENT;
    > >> + else
    > >> + ret = probe_file__get_events(kfd, filter, klist);
    > >> +
    > >> if (ret == 0) {
    > >> strlist__for_each(ent, klist)
    > >> pr_info("Removed event: %s\n", ent->s);
    > >> @@ -394,7 +398,11 @@ static int perf_del_probe_events(struct strfilter *filter)
    > >> goto error;
    > >> }
    > >>
    > >> - ret2 = probe_file__get_events(ufd, filter, ulist);
    > >> + if (ufd < 0)
    > >> + ret2 = -ENOENT;
    > >> + else
    > >> + ret2 = probe_file__get_events(ufd, filter, ulist);
    > >> +
    > >> if (ret2 == 0) {
    > >> strlist__for_each(ent, ulist)
    > >> pr_info("Removed event: %s\n", ent->s);
    > >> --
    > >> 1.8.3.4


    \
     
     \ /
      Last update: 2015-11-05 17:21    [W:3.223 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site