Re: [PATCH v1 11/11] perf cs-etm: Smatch: Fix potential NULL pointer dereference

Mathieu Poirier
2019-07-02T17:03:12Z

Hi Leo,

On Tue, Jul 02, 2019 at 06:34:20PM +0800, Leo Yan wrote:
> Based on the following report from Smatch, fix the potential
> NULL pointer dereference check.
>
> tools/perf/util/cs-etm.c:2545
> cs_etm__process_auxtrace_info() error: we previously assumed
> 'session->itrace_synth_opts' could be null (see line 2541)
>
> tools/perf/util/cs-etm.c
> 2541 if (session->itrace_synth_opts && session->itrace_synth_opts->set) {
> 2542 etm->synth_opts = *session->itrace_synth_opts;
> 2543 } else {
> 2544 itrace_synth_opts__set_default(&etm->synth_opts,
> 2545 session->itrace_synth_opts->default_no_sample);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 2546 etm->synth_opts.callchain = false;
> 2547 }
>
> To dismiss the potential NULL pointer dereference, this patch validates
> the pointer 'session->itrace_synth_opts' before access its elements.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/cs-etm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 0c7776b51045..b79df56eb9df 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -2540,7 +2540,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>
> if (session->itrace_synth_opts && session->itrace_synth_opts->set) {
> etm->synth_opts = *session->itrace_synth_opts;
> - } else {
> + } else if (session->itrace_synth_opts) {

This will work but we end up checking session->itrace_synth_opts twice. I
suggest to check it once and then process with the if/else if valid:

if (session->itrace_synth_opts) {
if (session->itrace_synth_opts->set) {
...
} else {
...
}
}

Thanks,
Mathieu

> itrace_synth_opts__set_default(&etm->synth_opts,
> session->itrace_synth_opts->default_no_sample);
> etm->synth_opts.callchain = false;
> --
> 2.17.1
>