* [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:53 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).