[PATCH] cpupower: idle_monitor: add NULL check after calloc()

[PATCH] cpupower: idle_monitor: add NULL check after calloc()

[longlong yan]

Twenty calloc() calls across five idle monitor files lack NULL return
checks, leading to potential NULL pointer dereferences on allocation
failure:

  - mperf_monitor.c mperf_register(): nine consecutive calloc() calls
    for per-CPU counters are used directly without any NULL check.
  - snb_idle.c snb_register(): is_valid and per-CSTATE previous/current
    counters are allocated without NULL checks.
  - nhm_idle.c intel_nhm_register(): same pattern as snb_idle.
  - hsw_ext_idle.c hsw_ext_register(): same pattern as snb_idle.
  - amd_fam14h_idle.c amd_fam14h_register(): per-state previous/current
    counters are allocated without NULL checks.

Add NULL checks after each allocation, returning NULL on failure,
consistent with existing error handling in the same functions (e.g.,
hardware capability checks already return NULL on failure).

Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
---
 tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c | 2 ++
 tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c   | 4 ++++
 tools/power/cpupower/utils/idle_monitor/mperf_monitor.c  | 5 +++++
 tools/power/cpupower/utils/idle_monitor/nhm_idle.c      | 4 ++++
 tools/power/cpupower/utils/idle_monitor/snb_idle.c       | 4 ++++
 5 files changed, 19 insertions(+)

diff --git a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
index 5edd35bd9ee9..6112161f7d5e 100644
--- a/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
+++ b/tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c
@@ -295,6 +295,8 @@ struct cpuidle_monitor *amd_fam14h_register(void)
 					      sizeof(unsigned long long));
 		current_count[num]  = calloc(cpu_count,
 					      sizeof(unsigned long long));
+		if (!previous_count[num] || !current_count[num])
+			return NULL;
 	}
 
 	/* We need PCI device: Slot 18, Func 6, compare with BKDG
diff --git a/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c b/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c
index f5a2a326b1b7..28cbe91b106b 100644
--- a/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c
+++ b/tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c
@@ -159,11 +159,15 @@ static struct cpuidle_monitor *hsw_ext_register(void)
 	}
 
 	is_valid = calloc(cpu_count, sizeof(int));
+	if (!is_valid)
+		return NULL;
 	for (num = 0; num < HSW_EXT_CSTATE_COUNT; num++) {
 		previous_count[num] = calloc(cpu_count,
 					sizeof(unsigned long long));
 		current_count[num]  = calloc(cpu_count,
 					sizeof(unsigned long long));
+		if (!previous_count[num] || !current_count[num])
+			return NULL;
 	}
 	intel_hsw_ext_monitor.name_len = strlen(intel_hsw_ext_monitor.name);
 	return &intel_hsw_ext_monitor;
diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
index 5ae02c3d5b64..5039716b591d 100644
--- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
+++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
@@ -350,6 +350,11 @@ struct cpuidle_monitor *mperf_register(void)
 	tsc_at_measure_end = calloc(cpu_count, sizeof(unsigned long long));
 	time_start = calloc(cpu_count, sizeof(struct timespec));
 	time_end = calloc(cpu_count, sizeof(struct timespec));
+	if (!is_valid || !mperf_previous_count || !aperf_previous_count ||
+	    !mperf_current_count || !aperf_current_count ||
+	    !tsc_at_measure_start || !tsc_at_measure_end ||
+	    !time_start || !time_end)
+		return NULL;
 	mperf_monitor.name_len = strlen(mperf_monitor.name);
 	return &mperf_monitor;
 }
diff --git a/tools/power/cpupower/utils/idle_monitor/nhm_idle.c b/tools/power/cpupower/utils/idle_monitor/nhm_idle.c
index 6b1733782ffa..ce745dac74cc 100644
--- a/tools/power/cpupower/utils/idle_monitor/nhm_idle.c
+++ b/tools/power/cpupower/utils/idle_monitor/nhm_idle.c
@@ -178,11 +178,15 @@ struct cpuidle_monitor *intel_nhm_register(void)
 
 	/* Free this at program termination */
 	is_valid = calloc(cpu_count, sizeof(int));
+	if (!is_valid)
+		return NULL;
 	for (num = 0; num < NHM_CSTATE_COUNT; num++) {
 		previous_count[num] = calloc(cpu_count,
 					sizeof(unsigned long long));
 		current_count[num]  = calloc(cpu_count,
 					sizeof(unsigned long long));
+		if (!previous_count[num] || !current_count[num])
+			return NULL;
 	}
 
 	intel_nhm_monitor.name_len = strlen(intel_nhm_monitor.name);
diff --git a/tools/power/cpupower/utils/idle_monitor/snb_idle.c b/tools/power/cpupower/utils/idle_monitor/snb_idle.c
index 5969b88a85b4..76d9986b0ecc 100644
--- a/tools/power/cpupower/utils/idle_monitor/snb_idle.c
+++ b/tools/power/cpupower/utils/idle_monitor/snb_idle.c
@@ -164,11 +164,15 @@ static struct cpuidle_monitor *snb_register(void)
 	}
 
 	is_valid = calloc(cpu_count, sizeof(int));
+	if (!is_valid)
+		return NULL;
 	for (num = 0; num < SNB_CSTATE_COUNT; num++) {
 		previous_count[num] = calloc(cpu_count,
 					sizeof(unsigned long long));
 		current_count[num]  = calloc(cpu_count,
 					sizeof(unsigned long long));
+		if (!previous_count[num] || !current_count[num])
+			return NULL;
 	}
 	intel_snb_monitor.name_len = strlen(intel_snb_monitor.name);
 	return &intel_snb_monitor;
-- 
2.43.0

Re: [PATCH] cpupower: idle_monitor: add NULL check after calloc()

[Shuah Khan]

On 9/2/26 00:52, longlong yan wrote:
> Twenty calloc() calls across five idle monitor files lack NULL return
> checks, leading to potential NULL pointer dereferences on allocation
> failure:
> 
>    - mperf_monitor.c mperf_register(): nine consecutive calloc() calls
>      for per-CPU counters are used directly without any NULL check.
>    - snb_idle.c snb_register(): is_valid and per-CSTATE previous/current
>      counters are allocated without NULL checks.
>    - nhm_idle.c intel_nhm_register(): same pattern as snb_idle.
>    - hsw_ext_idle.c hsw_ext_register(): same pattern as snb_idle.
>    - amd_fam14h_idle.c amd_fam14h_register(): per-state previous/current
>      counters are allocated without NULL checks.
> 
> Add NULL checks after each allocation, returning NULL on failure,
> consistent with existing error handling in the same functions (e.g.,
> hardware capability checks already return NULL on failure).

There is no need to add null checks for these small amounts
of memory from tools like this one.

How did you test these error paths?

Sorry I won't take this patch.

> 
> Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
> ---
>   tools/power/cpupower/utils/idle_monitor/amd_fam14h_idle.c | 2 ++
>   tools/power/cpupower/utils/idle_monitor/hsw_ext_idle.c   | 4 ++++
>   tools/power/cpupower/utils/idle_monitor/mperf_monitor.c  | 5 +++++
>   tools/power/cpupower/utils/idle_monitor/nhm_idle.c      | 4 ++++
>   tools/power/cpupower/utils/idle_monitor/snb_idle.c       | 4 ++++
>   5 files changed, 19 insertions(+)
> 

thanks,
-- Shuah