[PATCH] rtla/osnoise: fix NULL but dereferenced coccicheck errors

[PATCH] rtla/osnoise: fix NULL but dereferenced coccicheck errors

[Wan Jiabing]

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(-)

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/", &params->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/", &params->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);
-- 
2.35.1

Re: [PATCH] rtla/osnoise: fix NULL but dereferenced coccicheck errors

[Daniel Bristot de Oliveira]

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

Re: [PATCH] rtla/osnoise: fix NULL but dereferenced coccicheck errors

[Jiabing Wan]

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
>