2024-04-02 11:13:10

by Dawei Li

Subject: [PATCH 0/9] perf: Avoid explicit cpumask var allocation from stack

Hi,

This series try to eliminate direct cpumask var allocation from stack
for perf subsystem.

Direct/explicit allocation of cpumask on stack could be dangerous since
it can lead to stack overflow for systems with big NR_CPUS or
CONFIG_CPUMASK_OFFSTACK=y.

For arm64, it's more urgent since commit 3fbd56f0e7c1 ("ARM64: Dynamically
allocate cpumasks and increase supported CPUs to 512").

It's sort of a pattern that almost every cpumask var in perf subystem
occurs in teardown callback of cpuhp. In which case, if dynamic
allocation failed(which is unlikely), we choose return 0 rather than
-ENOMEM to caller cuz:
@teardown is not supposed to fail and if it does, system crashes:

static int cpuhp_issue_call(int cpu, enum cpuhp_state state, bool bringup,
struct hlist_node *node)
{
struct cpuhp_step *sp = cpuhp_get_step(state);
int ret;

/*
* If there's nothing to do, we done.
* Relies on the union for multi_instance.
*/
if (cpuhp_step_empty(bringup, sp))
return 0;
/*
* The non AP bound callbacks can fail on bringup. On teardown
* e.g. module removal we crash for now.
*/
#ifdef CONFIG_SMP
if (cpuhp_is_ap_state(state))
ret = cpuhp_invoke_ap_callback(cpu, state, bringup, node);
else
ret = cpuhp_invoke_callback(cpu, state, bringup, node,
NULL);
#else
ret = cpuhp_invoke_callback(cpu, state, bringup, node, NULL);
#endif
BUG_ON(ret && !bringup);
return ret;
}

Dawei Li (9):
perf/alibaba_uncore_drw: Avoid explicit cpumask var allocation from
stack
perf/arm-cmn: Avoid explicit cpumask var allocation from stack
perf/arm_cspmu: Avoid explicit cpumask var allocation from stack
perf/arm_dsu: Avoid explicit cpumask var allocation from stack
perf/dwc_pcie: Avoid explicit cpumask var allocation from stack
perf/hisi_pcie: Avoid explicit cpumask var allocation from stack
perf/hisi_uncore: Avoid explicit cpumask var allocation from stack
perf/qcom_l2: Avoid explicit cpumask var allocation from stack
perf/thunder_x2: Avoid explicit cpumask var allocation from stack

drivers/perf/alibaba_uncore_drw_pmu.c | 13 +++++++++----
drivers/perf/arm-cmn.c | 13 +++++++++----
drivers/perf/arm_cspmu/arm_cspmu.c | 13 +++++++++----
drivers/perf/arm_dsu_pmu.c | 18 +++++++++++++-----
drivers/perf/dwc_pcie_pmu.c | 17 +++++++++++------
drivers/perf/hisilicon/hisi_pcie_pmu.c | 15 ++++++++++-----
drivers/perf/hisilicon/hisi_uncore_pmu.c | 13 +++++++++----
drivers/perf/qcom_l2_pmu.c | 15 ++++++++++-----
drivers/perf/thunderx2_pmu.c | 20 ++++++++++++--------
9 files changed, 92 insertions(+), 45 deletions(-)


Thanks,

Dawei

--
2.27.0

2024-04-02 11:15:40

by Dawei Li

Subject: [PATCH 3/9] perf/arm_cspmu: Avoid explicit cpumask var allocation from stack

For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask
variable on stack is not recommended since it can cause potential stack
overflow.

Instead, kernel code should always use *cpumask_var API(s) to allocate
cpumask var in config- neutral way, leaving allocation strategy to
CONFIG_CPUMASK_OFFSTACK.

Use *cpumask_var API(s) to address it.

Signed-off-by: Dawei Li <[email protected]>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b9a252272f1e..8fa7c26aec28 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -1322,8 +1322,8 @@ static int arm_cspmu_cpu_online(unsigned int cpu, struct hlist_node *node)

static int arm_cspmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
{
+ cpumask_var_t online_supported;
int dst;
- struct cpumask online_supported;

struct arm_cspmu *cspmu =
hlist_entry_safe(node, struct arm_cspmu, cpuhp_node);
@@ -1332,17 +1332,22 @@ static int arm_cspmu_cpu_teardown(unsigned int cpu, struct hlist_node *node)
if (!cpumask_test_and_clear_cpu(cpu, &cspmu->active_cpu))
return 0;

+ if (!alloc_cpumask_var(&online_supported, GFP_KERNEL))
+ return 0;
+
/* Choose a new CPU to migrate ownership of the PMU to */
- cpumask_and(&online_supported, &cspmu->associated_cpus,
+ cpumask_and(online_supported, &cspmu->associated_cpus,
cpu_online_mask);
- dst = cpumask_any_but(&online_supported, cpu);
+ dst = cpumask_any_but(online_supported, cpu);
if (dst >= nr_cpu_ids)
- return 0;
+ goto __free_cpumask;

/* Use this CPU for event counting */
perf_pmu_migrate_context(&cspmu->pmu, cpu, dst);
arm_cspmu_set_active_cpu(dst, cspmu);

+__free_cpumask:
+ free_cpumask_var(online_supported);
return 0;
}

--
2.27.0