* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.