[bug report] perf env: Add perf_env__cpuid, perf_env__{nr_}pmu_mappings

[bug report] perf env: Add perf_env__cpuid, perf_env__{nr_}pmu_mappings

[Dan Carpenter]

Hello Kim Phillips,

This is a semi-automatic email about new static checker warnings.

The patch 9fe8895a27a8: "perf env: Add perf_env__cpuid,
perf_env__{nr_}pmu_mappings" from Aug 17, 2021, leads to the
following Smatch complaint:

    ./tools/perf/util/env.c:476 perf_env__nr_pmu_mappings()
    error: we previously assumed 'env' could be null (see line 470)

./tools/perf/util/env.c
   469	
   470		if (!env || !env->nr_pmu_mappings) { /* Assume local operation */
                     ^^^
Checks for NULL

   471			status = perf_env__read_pmu_mappings(env);
   472			if (status)
   473				return 0;
   474		}
   475	
   476		return env->nr_pmu_mappings;
                       ^^^^
Unchecked dereference

   477	}

regards,
dan carpenter

Re: [bug report] perf env: Add perf_env__cpuid, perf_env__{nr_}pmu_mappings

[Jiri Olsa]

On Tue, Feb 08, 2022 at 02:22:39PM +0300, Dan Carpenter wrote:
> Hello Kim Phillips,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 9fe8895a27a8: "perf env: Add perf_env__cpuid,
> perf_env__{nr_}pmu_mappings" from Aug 17, 2021, leads to the
> following Smatch complaint:
> 
>     ./tools/perf/util/env.c:476 perf_env__nr_pmu_mappings()
>     error: we previously assumed 'env' could be null (see line 470)
> 
> ./tools/perf/util/env.c
>    469	
>    470		if (!env || !env->nr_pmu_mappings) { /* Assume local operation */
>                      ^^^
> Checks for NULL
> 
>    471			status = perf_env__read_pmu_mappings(env);
>    472			if (status)
>    473				return 0;
>    474		}
>    475	
>    476		return env->nr_pmu_mappings;
>                        ^^^^
> Unchecked dereference
> 
>    477	}
> 
> regards,
> dan carpenter

I think we need to bail out for !env

jirka


---
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 579e44c59914..bc30d8cd2790 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -467,7 +467,10 @@ int perf_env__nr_pmu_mappings(struct perf_env *env)
 {
 	int status;
 
-	if (!env || !env->nr_pmu_mappings) { /* Assume local operation */
+	if (!env)
+		return 0;
+
+	if (!env->nr_pmu_mappings) { /* Assume local operation */
 		status = perf_env__read_pmu_mappings(env);
 		if (status)
 			return 0;
@@ -480,7 +483,10 @@ const char *perf_env__pmu_mappings(struct perf_env *env)
 {
 	int status;
 
-	if (!env || !env->pmu_mappings) { /* Assume local operation */
+	if (!env)
+		return NULL;
+
+	if (!env->pmu_mappings) { /* Assume local operation */
 		status = perf_env__read_pmu_mappings(env);
 		if (status)
 			return NULL;

[bug report] perf env: Add perf_env__cpuid, perf_env__{nr_}pmu_mappings

[Dan Carpenter]

Hello Kim Phillips,

This is a semi-automatic email about new static checker warnings.

The patch 9fe8895a27a8: "perf env: Add perf_env__cpuid,
perf_env__{nr_}pmu_mappings" from Aug 17, 2021, leads to the
following Smatch complaint:

    tools/perf/util/env.c:467 perf_env__cpuid()
    error: we previously assumed 'env' could be null (see line 461)

tools/perf/util/env.c
   457  const char *perf_env__cpuid(struct perf_env *env)
   458  {
   459          int status;
   460  
   461          if (!env || !env->cpuid) { /* Assume local operation */
   462                  status = perf_env__read_cpuid(env);
   463                  if (status)
   464                          return NULL;

If env is NULL and perf_env__read_cpuid() return zero then we are
toasted.

   465          }
   466  
   467          return env->cpuid;
   468  }

A couple of the neighboring functions are written the same way.  I'm not
sure why they don't generate checker warnings.

regards,
dan carpenter