* [PATCH] perf session: check for null pointer before derefernce
@ 2022-01-24 15:00 Ameer Hamza
2022-01-24 15:30 ` James Clark
0 siblings, 1 reply; 6+ messages in thread
From: Ameer Hamza @ 2022-01-24 15:00 UTC (permalink / raw)
To: mark.rutland, alexander.shishkin, jolsa, namhyung
Cc: peterz, mingo, acme, rickyman7, alexey.v.bayduraev, adrian.hunter,
leo.yan, german.gomez, linux-perf-users, linux-kernel,
amhamza.mgc
Move null pointer check before dereferncing the variable
Addresses-Coverity: 1497622 ("Derereference before null check")
Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com>
---
tools/perf/util/session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f19348dddd55..e1014ab62c10 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1503,11 +1503,11 @@ static int machines__deliver_event(struct machines *machines,
++evlist->stats.nr_unknown_id;
return 0;
}
- dump_sample(evsel, event, sample, perf_env__arch(machine->env));
if (machine == NULL) {
++evlist->stats.nr_unprocessable_samples;
return 0;
}
+ dump_sample(evsel, event, sample, perf_env__arch(machine->env));
return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine);
case PERF_RECORD_MMAP:
return tool->mmap(tool, event, sample, machine);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] perf session: check for null pointer before derefernce 2022-01-24 15:00 [PATCH] perf session: check for null pointer before derefernce Ameer Hamza @ 2022-01-24 15:30 ` James Clark 2022-01-24 20:29 ` Ameer Hamza 0 siblings, 1 reply; 6+ messages in thread From: James Clark @ 2022-01-24 15:30 UTC (permalink / raw) To: Ameer Hamza, German Gomez Cc: peterz, mingo, acme, rickyman7, alexey.v.bayduraev, adrian.hunter, leo.yan, linux-perf-users, linux-kernel, mark.rutland, alexander.shishkin, jolsa, namhyung On 24/01/2022 15:00, Ameer Hamza wrote: > Move null pointer check before dereferncing the variable > > Addresses-Coverity: 1497622 ("Derereference before null check") > > Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> > --- > tools/perf/util/session.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index f19348dddd55..e1014ab62c10 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -1503,11 +1503,11 @@ static int machines__deliver_event(struct machines *machines, > ++evlist->stats.nr_unknown_id; > return 0; > } > - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > if (machine == NULL) { > ++evlist->stats.nr_unprocessable_samples; > return 0; > } > + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); > case PERF_RECORD_MMAP: > return tool->mmap(tool, event, sample, machine); > Hi Ameer, This mistake was made recently, but I don't think your change is the desired behavior. It should be possible to dump stuff if machine is null. null or an empty string should be passed to dump_sample(). It looks like some of the printfs still attempt to show something in this case, although I didn't check them all. James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf session: check for null pointer before derefernce 2022-01-24 15:30 ` James Clark @ 2022-01-24 20:29 ` Ameer Hamza 2022-01-25 9:35 ` James Clark 0 siblings, 1 reply; 6+ messages in thread From: Ameer Hamza @ 2022-01-24 20:29 UTC (permalink / raw) To: James Clark Cc: German Gomez, peterz, mingo, acme, rickyman7, alexey.v.bayduraev, adrian.hunter, leo.yan, linux-perf-users, linux-kernel, mark.rutland, alexander.shishkin, jolsa, namhyung On Mon, Jan 24, 2022 at 03:30:17PM +0000, James Clark wrote: > > > On 24/01/2022 15:00, Ameer Hamza wrote: > > Move null pointer check before dereferncing the variable > > > > Addresses-Coverity: 1497622 ("Derereference before null check") > > > > Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> > > --- > > tools/perf/util/session.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > > index f19348dddd55..e1014ab62c10 100644 > > --- a/tools/perf/util/session.c > > +++ b/tools/perf/util/session.c > > @@ -1503,11 +1503,11 @@ static int machines__deliver_event(struct machines *machines, > > ++evlist->stats.nr_unknown_id; > > return 0; > > } > > - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > > if (machine == NULL) { > > ++evlist->stats.nr_unprocessable_samples; > > return 0; > > } > > + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > > return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); > > case PERF_RECORD_MMAP: > > return tool->mmap(tool, event, sample, machine); > > > > Hi Ameer, > > This mistake was made recently, but I don't think your change is the desired behavior. > > It should be possible to dump stuff if machine is null. null or an empty string > should be passed to dump_sample(). It looks like some of the printfs still attempt to > show something in this case, although I didn't check them all. Hi James, Thank you for your response. I understand your point. Do you think following changes would be ok? diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index f19348dddd55..210eeee3dd70 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1503,11 +1503,12 @@ static int machines__deliver_event(struct machines *machines, ++evlist->stats.nr_unknown_id; return 0; } - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); if (machine == NULL) { ++evlist->stats.nr_unprocessable_samples; + dump_sample(evsel, event, sample, perf_env__arch(NULL)); return 0; } + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); case PERF_RECORD_MMAP: return tool->mmap(tool, event, sample, machine); Thanks, Hamza ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf session: check for null pointer before derefernce 2022-01-24 20:29 ` Ameer Hamza @ 2022-01-25 9:35 ` James Clark 2022-01-25 12:11 ` [PATCH v2] " Ameer Hamza 2022-02-06 11:41 ` [PATCH] " Arnaldo Carvalho de Melo 0 siblings, 2 replies; 6+ messages in thread From: James Clark @ 2022-01-25 9:35 UTC (permalink / raw) To: Ameer Hamza Cc: German Gomez, peterz, mingo, acme, rickyman7, alexey.v.bayduraev, adrian.hunter, leo.yan, linux-perf-users, linux-kernel, mark.rutland, alexander.shishkin, jolsa, namhyung On 24/01/2022 20:29, Ameer Hamza wrote: > On Mon, Jan 24, 2022 at 03:30:17PM +0000, James Clark wrote: >> >> >> On 24/01/2022 15:00, Ameer Hamza wrote: >>> Move null pointer check before dereferncing the variable >>> >>> Addresses-Coverity: 1497622 ("Derereference before null check") >>> >>> Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> >>> --- >>> tools/perf/util/session.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c >>> index f19348dddd55..e1014ab62c10 100644 >>> --- a/tools/perf/util/session.c >>> +++ b/tools/perf/util/session.c >>> @@ -1503,11 +1503,11 @@ static int machines__deliver_event(struct machines *machines, >>> ++evlist->stats.nr_unknown_id; >>> return 0; >>> } >>> - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); >>> if (machine == NULL) { >>> ++evlist->stats.nr_unprocessable_samples; >>> return 0; >>> } >>> + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); >>> return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); >>> case PERF_RECORD_MMAP: >>> return tool->mmap(tool, event, sample, machine); >>> >> >> Hi Ameer, >> >> This mistake was made recently, but I don't think your change is the desired behavior. >> >> It should be possible to dump stuff if machine is null. null or an empty string >> should be passed to dump_sample(). It looks like some of the printfs still attempt to >> show something in this case, although I didn't check them all. > > Hi James, > > Thank you for your response. I understand your point. > > Do you think following changes would be ok? Yep looks good. With that change: Reviewed-by: James Clark <james.clark@arm.com> > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index f19348dddd55..210eeee3dd70 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -1503,11 +1503,12 @@ static int machines__deliver_event(struct machines *machines, > ++evlist->stats.nr_unknown_id; > return 0; > } > - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > if (machine == NULL) { > ++evlist->stats.nr_unprocessable_samples; > + dump_sample(evsel, event, sample, perf_env__arch(NULL)); > return 0; > } > + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); > case PERF_RECORD_MMAP: > return tool->mmap(tool, event, sample, machine); > > Thanks, > Hamza > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] perf session: check for null pointer before derefernce 2022-01-25 9:35 ` James Clark @ 2022-01-25 12:11 ` Ameer Hamza 2022-02-06 11:41 ` [PATCH] " Arnaldo Carvalho de Melo 1 sibling, 0 replies; 6+ messages in thread From: Ameer Hamza @ 2022-01-25 12:11 UTC (permalink / raw) To: mark.rutland, alexander.shishkin, jolsa, namhyung Cc: peterz, mingo, acme, rickyman7, alexey.v.bayduraev, adrian.hunter, leo.yan, german.gomez, linux-perf-users, linux-kernel, amhamza.mgc, James Clark Move null pointer check before dereferncing the variable Addresses-Coverity: 1497622 ("Derereference before null check") Reviewed-by: James Clark <james.clark@arm.com> Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> --- v1 -> v2: It should be possible to dump stuff if machine is NULL. --- tools/perf/util/session.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index f19348dddd55..210eeee3dd70 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1503,11 +1503,12 @@ static int machines__deliver_event(struct machines *machines, ++evlist->stats.nr_unknown_id; return 0; } - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); if (machine == NULL) { ++evlist->stats.nr_unprocessable_samples; + dump_sample(evsel, event, sample, perf_env__arch(NULL)); return 0; } + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); case PERF_RECORD_MMAP: return tool->mmap(tool, event, sample, machine); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf session: check for null pointer before derefernce 2022-01-25 9:35 ` James Clark 2022-01-25 12:11 ` [PATCH v2] " Ameer Hamza @ 2022-02-06 11:41 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2022-02-06 11:41 UTC (permalink / raw) To: James Clark Cc: Ameer Hamza, German Gomez, peterz, mingo, rickyman7, alexey.v.bayduraev, adrian.hunter, leo.yan, linux-perf-users, linux-kernel, mark.rutland, alexander.shishkin, jolsa, namhyung Em Tue, Jan 25, 2022 at 09:35:49AM +0000, James Clark escreveu: > > > On 24/01/2022 20:29, Ameer Hamza wrote: > > On Mon, Jan 24, 2022 at 03:30:17PM +0000, James Clark wrote: > >> > >> > >> On 24/01/2022 15:00, Ameer Hamza wrote: > >>> Move null pointer check before dereferncing the variable > >>> > >>> Addresses-Coverity: 1497622 ("Derereference before null check") > >>> > >>> Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> > >>> --- > >>> tools/perf/util/session.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > >>> index f19348dddd55..e1014ab62c10 100644 > >>> --- a/tools/perf/util/session.c > >>> +++ b/tools/perf/util/session.c > >>> @@ -1503,11 +1503,11 @@ static int machines__deliver_event(struct machines *machines, > >>> ++evlist->stats.nr_unknown_id; > >>> return 0; > >>> } > >>> - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > >>> if (machine == NULL) { > >>> ++evlist->stats.nr_unprocessable_samples; > >>> return 0; > >>> } > >>> + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > >>> return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); > >>> case PERF_RECORD_MMAP: > >>> return tool->mmap(tool, event, sample, machine); > >>> > >> > >> Hi Ameer, > >> > >> This mistake was made recently, but I don't think your change is the desired behavior. > >> > >> It should be possible to dump stuff if machine is null. null or an empty string > >> should be passed to dump_sample(). It looks like some of the printfs still attempt to > >> show something in this case, although I didn't check them all. > > > > Hi James, > > > > Thank you for your response. I understand your point. > > > > Do you think following changes would be ok? > > Yep looks good. With that change: > > Reviewed-by: James Clark <james.clark@arm.com> Thanks, applied. - Arnaldo > > > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > > index f19348dddd55..210eeee3dd70 100644 > > --- a/tools/perf/util/session.c > > +++ b/tools/perf/util/session.c > > @@ -1503,11 +1503,12 @@ static int machines__deliver_event(struct machines *machines, > > ++evlist->stats.nr_unknown_id; > > return 0; > > } > > - dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > > if (machine == NULL) { > > ++evlist->stats.nr_unprocessable_samples; > > + dump_sample(evsel, event, sample, perf_env__arch(NULL)); > > return 0; > > } > > + dump_sample(evsel, event, sample, perf_env__arch(machine->env)); > > return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine); > > case PERF_RECORD_MMAP: > > return tool->mmap(tool, event, sample, machine); > > > > Thanks, > > Hamza > > -- - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-06 11:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-24 15:00 [PATCH] perf session: check for null pointer before derefernce Ameer Hamza 2022-01-24 15:30 ` James Clark 2022-01-24 20:29 ` Ameer Hamza 2022-01-25 9:35 ` James Clark 2022-01-25 12:11 ` [PATCH v2] " Ameer Hamza 2022-02-06 11:41 ` [PATCH] " Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).