From: Bo Yu <[email protected]>
There is a freed pointer "evsel", so fix it.
Detected by CoverityScan, CID#1442595("Memory-illegalaccesses
(USE_AFTER_FREE)")
Fixes: 6ab3bc240ade4("perf trace: Support multiple "vfs_getname" probes")
Signed-off-by: Bo Yu <[email protected]>
---
tools/perf/builtin-trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index b36061cd1ab8..4036b20a1067 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2515,7 +2515,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp);
static bool perf_evlist__add_vfs_getname(struct perf_evlist *evlist)
{
bool found = false;
- struct perf_evsel *evsel, *tmp;
+ struct perf_evsel *evsel = NULL, *tmp;
struct parse_events_error err = { .idx = 0, };
int ret = parse_events(evlist, "probe:vfs_getname*", &err);
--
2.11.0
On Thu, Feb 14, 2019 at 12:23:56AM -0500, Bo YU wrote:
> From: Bo Yu <[email protected]>
>
> There is a freed pointer "evsel", so fix it.
>
> Detected by CoverityScan, CID#1442595("Memory-illegalaccesses
> (USE_AFTER_FREE)")
> Fixes: 6ab3bc240ade4("perf trace: Support multiple "vfs_getname" probes")
>
> Signed-off-by: Bo Yu <[email protected]>
> ---
> tools/perf/builtin-trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index b36061cd1ab8..4036b20a1067 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2515,7 +2515,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp);
> static bool perf_evlist__add_vfs_getname(struct perf_evlist *evlist)
> {
> bool found = false;
> - struct perf_evsel *evsel, *tmp;
> + struct perf_evsel *evsel = NULL, *tmp;
hum, I can't see how this change could matter,
could you pelase explain?
jirka
> struct parse_events_error err = { .idx = 0, };
> int ret = parse_events(evlist, "probe:vfs_getname*", &err);
>
> --
> 2.11.0
>
Hi,
On Thu, Feb 14, 2019 at 09:34:11AM +0100, Jiri Olsa wrote:
>On Thu, Feb 14, 2019 at 12:23:56AM -0500, Bo YU wrote:
>> From: Bo Yu <[email protected]>
>>
>> There is a freed pointer "evsel", so fix it.
>>
>> Detected by CoverityScan, CID#1442595("Memory-illegalaccesses
>> (USE_AFTER_FREE)")
>> Fixes: 6ab3bc240ade4("perf trace: Support multiple "vfs_getname" probes")
>>
>> Signed-off-by: Bo Yu <[email protected]>
>> ---
>> tools/perf/builtin-trace.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
>> index b36061cd1ab8..4036b20a1067 100644
>> --- a/tools/perf/builtin-trace.c
>> +++ b/tools/perf/builtin-trace.c
>> @@ -2515,7 +2515,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp);
>> static bool perf_evlist__add_vfs_getname(struct perf_evlist *evlist)
>> {
>> bool found = false;
>> - struct perf_evsel *evsel, *tmp;
>> + struct perf_evsel *evsel = NULL, *tmp;
>
>hum, I can't see how this change could matter,
>could you pelase explain
First, this is a warning reported by CoverityScan,but in fact i do not how
to answer your question :(.
Second, if i remember right, temporary element of list_for_each_entry_safe
should be initialized with NULL otherwise it will complain via gcc.
Please correct me :)
Thanks,
>
>jirka
>
>> struct parse_events_error err = { .idx = 0, };
>> int ret = parse_events(evlist, "probe:vfs_getname*", &err);
>>
>> --
>> 2.11.0
>>
On Thu, Feb 14, 2019 at 05:22:39AM -0500, YU Bo wrote:
> Hi,
> On Thu, Feb 14, 2019 at 09:34:11AM +0100, Jiri Olsa wrote:
> > On Thu, Feb 14, 2019 at 12:23:56AM -0500, Bo YU wrote:
> > > From: Bo Yu <[email protected]>
> > >
> > > There is a freed pointer "evsel", so fix it.
> > >
> > > Detected by CoverityScan, CID#1442595("Memory-illegalaccesses
> > > (USE_AFTER_FREE)")
> > > Fixes: 6ab3bc240ade4("perf trace: Support multiple "vfs_getname" probes")
> > >
> > > Signed-off-by: Bo Yu <[email protected]>
> > > ---
> > > tools/perf/builtin-trace.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > > index b36061cd1ab8..4036b20a1067 100644
> > > --- a/tools/perf/builtin-trace.c
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -2515,7 +2515,7 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp);
> > > static bool perf_evlist__add_vfs_getname(struct perf_evlist *evlist)
> > > {
> > > bool found = false;
> > > - struct perf_evsel *evsel, *tmp;
> > > + struct perf_evsel *evsel = NULL, *tmp;
> >
> > hum, I can't see how this change could matter,
> > could you pelase explain
> First, this is a warning reported by CoverityScan,but in fact i do not how
> to answer your question :(.
I understand that, however at the same time I think
it's good to have an idea what the patch is doing ;-)
> Second, if i remember right, temporary element of list_for_each_entry_safe
> should be initialized with NULL otherwise it will complain via gcc.
> Please correct me :)
hum, from quick look:
perf_evlist__add_vfs_getname
struct perf_evsel *evsel;
evlist__for_each_entry_safe(evlist, evsel, tmp)
-> __evlist__for_each_entry_safe(&(evlist)->entries, tmp, evsel)
__evlist__for_each_entry_safe(list, tmp, evsel) \
-> list_for_each_entry_safe(evsel, tmp, list, node)
list_for_each_entry_safe(pos, n, head, member) \
-> for (pos = list_first_entry(head, typeof(*pos), member), \
n = list_next_entry(pos, member); \
&pos->member != (head); \
pos = n, n = list_next_entry(n, member))
unless I'm missing something 'evsel' is being initialized
in the for loop init section with this statement:
pos = list_first_entry(head, typeof(*pos), member)
jirka