* [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 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.