| Message ID | 20220322113253.57390-1-wanjiabing@vivo.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rtla/osnoise: fix NULL but dereferenced coccicheck errors | expand |
Hi Wan On 3/22/22 12:32, Wan Jiabing wrote: > Fix the following coccicheck errors: > ./tools/tracing/rtla/src/osnoise_hist.c:870:31-36: ERROR: record is NULL but dereferenced. > ./tools/tracing/rtla/src/osnoise_top.c:650:31-36: ERROR: record is NULL but dereferenced. > > "record" is NULL before calling osnoise_init_trace_tool. > Add a tag "out_free" to avoid dereferring a NULL pointer. Yes, I see the problem. Please, change the Subject to something with better information about the problem, instead of the way you fund it. > Fixes: 51d64c3a1819 ("rtla: Add -e/--event support") > Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> > --- > tools/tracing/rtla/src/osnoise_hist.c | 5 +++-- > tools/tracing/rtla/src/osnoise_top.c | 9 +++++---- > 2 files changed, 8 insertions(+), 6 deletions(-) Please, also add the version of coccicheck you are using, because it seems that there is a problem with it. There is a similar problem with timerlat, added by the same patch, but it seems that coccicheck did not find it. Would you mind to check coccicheck, and also add the fix for timerlat? Thanks. -- Daniel
Hi Daniel, On 2022/3/31 20:25, Daniel Bristot de Oliveira wrote: > Hi Wan > > On 3/22/22 12:32, Wan Jiabing wrote: >> Fix the following coccicheck errors: >> ./tools/tracing/rtla/src/osnoise_hist.c:870:31-36: ERROR: record is NULL but dereferenced. >> ./tools/tracing/rtla/src/osnoise_top.c:650:31-36: ERROR: record is NULL but dereferenced. >> >> "record" is NULL before calling osnoise_init_trace_tool. >> Add a tag "out_free" to avoid dereferring a NULL pointer. > Yes, I see the problem. > > Please, change the Subject to something with better information about the > problem, instead of the way you fund it. OK, I'll fix it. >> Fixes: 51d64c3a1819 ("rtla: Add -e/--event support") >> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >> --- >> tools/tracing/rtla/src/osnoise_hist.c | 5 +++-- >> tools/tracing/rtla/src/osnoise_top.c | 9 +++++---- >> 2 files changed, 8 insertions(+), 6 deletions(-) > Please, also add the version of coccicheck you are using, because it seems that > there is a problem with it. There is a similar problem with timerlat, added by > the same patch, but it seems that coccicheck did not find it. > > Would you mind to check coccicheck, and also add the fix for timerlat? Yes, I used null/deref_null.cocci to report these warnings. But there were no warnings in timerlat. So I check the null/deref_null.cocci and modify some rules. Using modified null/deref_null.cocci, there are warnings as follow: tools/tracing/rtla/src/timerlat_hist.c:905:31-36: ERROR: record is NULL but dereferenced. tools/tracing/rtla/src/osnoise_top.c:650:31-36: ERROR: record is NULL but dereferenced. tools/tracing/rtla/src/osnoise_hist.c:870:31-36: ERROR: record is NULL but dereferenced. tools/tracing/rtla/src/timerlat_top.c:700:31-36: ERROR: record is NULL but dereferenced. I'll resend the patch and fix them all. Thanks, Wan Jiabing > Thanks. > -- Daniel >
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index b4380d45cacd..5d7ea479ac89 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -809,7 +809,7 @@ int osnoise_hist_main(int argc, char *argv[]) retval = set_comm_sched_attr("osnoise/", ¶ms->sched_param); if (retval) { err_msg("Failed to set sched parameters\n"); - goto out_hist; + goto out_free; } } @@ -819,7 +819,7 @@ int osnoise_hist_main(int argc, char *argv[]) record = osnoise_init_trace_tool("osnoise"); if (!record) { err_msg("Failed to enable the trace instance\n"); - goto out_hist; + goto out_free; } if (params->events) { @@ -869,6 +869,7 @@ int osnoise_hist_main(int argc, char *argv[]) out_hist: trace_events_destroy(&record->trace, params->events); params->events = NULL; +out_free: osnoise_free_histogram(tool->data); out_destroy: osnoise_destroy_tool(record); diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 72c2fd6ce005..76479bfb2922 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -572,7 +572,7 @@ int osnoise_top_main(int argc, char **argv) retval = osnoise_top_apply_config(tool, params); if (retval) { err_msg("Could not apply config\n"); - goto out_top; + goto out_free; } trace = &tool->trace; @@ -580,14 +580,14 @@ int osnoise_top_main(int argc, char **argv) retval = enable_osnoise(trace); if (retval) { err_msg("Failed to enable osnoise tracer\n"); - goto out_top; + goto out_free; } if (params->set_sched) { retval = set_comm_sched_attr("osnoise/", ¶ms->sched_param); if (retval) { err_msg("Failed to set sched parameters\n"); - goto out_top; + goto out_free; } } @@ -597,7 +597,7 @@ int osnoise_top_main(int argc, char **argv) record = osnoise_init_trace_tool("osnoise"); if (!record) { err_msg("Failed to enable the trace instance\n"); - goto out_top; + goto out_free; } if (params->events) { @@ -649,6 +649,7 @@ int osnoise_top_main(int argc, char **argv) out_top: trace_events_destroy(&record->trace, params->events); params->events = NULL; +out_free: osnoise_free_top(tool->data); osnoise_destroy_tool(record); osnoise_destroy_tool(tool);
Fix the following coccicheck errors: ./tools/tracing/rtla/src/osnoise_hist.c:870:31-36: ERROR: record is NULL but dereferenced. ./tools/tracing/rtla/src/osnoise_top.c:650:31-36: ERROR: record is NULL but dereferenced. "record" is NULL before calling osnoise_init_trace_tool. Add a tag "out_free" to avoid dereferring a NULL pointer. Fixes: 51d64c3a1819 ("rtla: Add -e/--event support") Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> --- tools/tracing/rtla/src/osnoise_hist.c | 5 +++-- tools/tracing/rtla/src/osnoise_top.c | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-)