| Message ID | 20250410112027.80131-1-hanchunchao@inspur.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drivers/perf: Add NULL check in thunderx2_pmu | expand |
On Thu, Apr 10, 2025 at 07:20:27PM +0800, Charles Han wrote: > devm_kasprintf() can return a NULL pointer on failure,but this > returned value in tx2_uncore_pmu_register() and > tx2_uncore_pmu_init_dev() is not checked. > Add NULL check in tx2_uncore_pmu_register() and > tx2_uncore_pmu_init_dev(), to handle kernel NULL > pointer dereference error. > > Fixes: 69c32972d593 ("drivers/perf: Add Cavium ThunderX2 SoC UNCORE PMU driver") > Fixes: 5e2c27e833bb ("drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.") > Signed-off-by: Charles Han <hanchunchao@inspur.com> > --- > drivers/perf/thunderx2_pmu.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c > index 6ed4707bd6bb..03dd297ff326 100644 > --- a/drivers/perf/thunderx2_pmu.c > +++ b/drivers/perf/thunderx2_pmu.c > @@ -738,6 +738,8 @@ static int tx2_uncore_pmu_register( > > tx2_pmu->pmu.name = devm_kasprintf(dev, GFP_KERNEL, > "%s", name); > + if (!tx2_pmu->pmu.name) > + return -ENOMEM; perf_pmu_register() rejects (and warns) for a NULL name, so I'm not sure we need to litter the drivers with extra checks. > return perf_pmu_register(&tx2_pmu->pmu, tx2_pmu->pmu.name, -1); > } > @@ -837,6 +839,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, > tx2_pmu->attr_groups = l3c_pmu_attr_groups; > tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, > "uncore_l3c_%d", tx2_pmu->node); > + if (!tx2_pmu->name) { > + devm_kfree(dev, tx2_pmu); > + return ERR_PTR(-ENOMEM); > + } > + > tx2_pmu->init_cntr_base = init_cntr_base_l3c; > tx2_pmu->start_event = uncore_start_event_l3c; > tx2_pmu->stop_event = uncore_stop_event_l3c; > @@ -852,6 +859,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, > tx2_pmu->attr_groups = dmc_pmu_attr_groups; > tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, > "uncore_dmc_%d", tx2_pmu->node); > + if (!tx2_pmu->name) { > + devm_kfree(dev, tx2_pmu); > + return ERR_PTR(-ENOMEM); > + } > + > tx2_pmu->init_cntr_base = init_cntr_base_dmc; > tx2_pmu->start_event = uncore_start_event_dmc; > tx2_pmu->stop_event = uncore_stop_event_dmc; > @@ -866,6 +878,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, > tx2_pmu->attr_groups = ccpi2_pmu_attr_groups; > tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, > "uncore_ccpi2_%d", tx2_pmu->node); > + if (!tx2_pmu->name) { > + devm_kfree(dev, tx2_pmu); > + return ERR_PTR(-ENOMEM); > + } Are any of these actually necessary? afaict, the vsprintf code will emit "(null)" for a NULL pointer. Will
diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c index 6ed4707bd6bb..03dd297ff326 100644 --- a/drivers/perf/thunderx2_pmu.c +++ b/drivers/perf/thunderx2_pmu.c @@ -738,6 +738,8 @@ static int tx2_uncore_pmu_register( tx2_pmu->pmu.name = devm_kasprintf(dev, GFP_KERNEL, "%s", name); + if (!tx2_pmu->pmu.name) + return -ENOMEM; return perf_pmu_register(&tx2_pmu->pmu, tx2_pmu->pmu.name, -1); } @@ -837,6 +839,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, tx2_pmu->attr_groups = l3c_pmu_attr_groups; tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, "uncore_l3c_%d", tx2_pmu->node); + if (!tx2_pmu->name) { + devm_kfree(dev, tx2_pmu); + return ERR_PTR(-ENOMEM); + } + tx2_pmu->init_cntr_base = init_cntr_base_l3c; tx2_pmu->start_event = uncore_start_event_l3c; tx2_pmu->stop_event = uncore_stop_event_l3c; @@ -852,6 +859,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, tx2_pmu->attr_groups = dmc_pmu_attr_groups; tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, "uncore_dmc_%d", tx2_pmu->node); + if (!tx2_pmu->name) { + devm_kfree(dev, tx2_pmu); + return ERR_PTR(-ENOMEM); + } + tx2_pmu->init_cntr_base = init_cntr_base_dmc; tx2_pmu->start_event = uncore_start_event_dmc; tx2_pmu->stop_event = uncore_stop_event_dmc; @@ -866,6 +878,11 @@ static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev, tx2_pmu->attr_groups = ccpi2_pmu_attr_groups; tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL, "uncore_ccpi2_%d", tx2_pmu->node); + if (!tx2_pmu->name) { + devm_kfree(dev, tx2_pmu); + return ERR_PTR(-ENOMEM); + } + tx2_pmu->init_cntr_base = init_cntr_base_ccpi2; tx2_pmu->start_event = uncore_start_event_ccpi2; tx2_pmu->stop_event = uncore_stop_event_ccpi2;
devm_kasprintf() can return a NULL pointer on failure,but this returned value in tx2_uncore_pmu_register() and tx2_uncore_pmu_init_dev() is not checked. Add NULL check in tx2_uncore_pmu_register() and tx2_uncore_pmu_init_dev(), to handle kernel NULL pointer dereference error. Fixes: 69c32972d593 ("drivers/perf: Add Cavium ThunderX2 SoC UNCORE PMU driver") Fixes: 5e2c27e833bb ("drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.") Signed-off-by: Charles Han <hanchunchao@inspur.com> --- drivers/perf/thunderx2_pmu.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)