Hi Kajol,
Thanks for the patch. Minor review comment below:
Kajol Jain <kj...@linux.ibm.com> writes:
> With CONFIG_FORTIFY_SOURCE enabled, string functions will also perform
> dynamic checks for string size which can panic the kernel,
> like incase of overflow detection.
>
> In papr_scm, papr_scm_pmu_check_events function uses stat->stat_id
> with string operations, to populate the nvdimm_events_map array.
> Since stat_id variable is not NULL terminated, the kernel panics
> with CONFIG_FORTIFY_SOURCE enabled at boot time.
>
> Below are the logs of kernel panic:
>
> [ 0.090221][ T1] detected buffer overflow in __fortify_strlen
> [ 0.090241][ T1] ------------[ cut here ]------------
> [ 0.090246][ T1] kernel BUG at lib/string_helpers.c:980!
> [ 0.090253][ T1] Oops: Exception in kernel mode, sig: 5 [#1]
> ........
> [ 0.090375][ T1] NIP [c00000000077dad0] fortify_panic+0x28/0x38
> [ 0.090382][ T1] LR [c00000000077dacc] fortify_panic+0x24/0x38
> [ 0.090387][ T1] Call Trace:
> [ 0.090390][ T1] [c0000022d77836e0] [c00000000077dacc]
> fortify_panic+0x24/0x38 (unreliable)
> [ 9.297707] [ T1] [c00800000deb2660]
> papr_scm_pmu_check_events.constprop.0+0x118/0x220 [papr_scm]
> [ 9.297721] [ T1] [c00800000deb2cb0] papr_scm_probe+0x288/0x62c
> [papr_scm]
> [ 9.297732] [ T1] [c0000000009b46a8] platform_probe+0x98/0x150
>
> Fix this issue by using kmemdup_nul function to copy the content of
> stat->stat_id directly to the nvdimm_events_map array.
>
> Fixes: 4c08d4bbc089 ("powerpc/papr_scm: Add perf interface support")
> Signed-off-by: Kajol Jain <kj...@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index f58728d5f10d..39962c905542 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -462,7 +462,6 @@ static int papr_scm_pmu_check_events(struct papr_scm_priv
> *p, struct nvdimm_pmu
> {
> struct papr_scm_perf_stat *stat;
> struct papr_scm_perf_stats *stats;
> - char *statid;
> int index, rc, count;
> u32 available_events;
>
> @@ -493,14 +492,12 @@ static int papr_scm_pmu_check_events(struct
> papr_scm_priv *p, struct nvdimm_pmu
>
> for (index = 0, stat = stats->scm_statistic, count = 0;
> index < available_events; index++, ++stat) {
> - statid = kzalloc(strlen(stat->stat_id) + 1, GFP_KERNEL);
> - if (!statid) {
> + p->nvdimm_events_map[count] = kmemdup_nul(stat->stat_id,
> 8, GFP_KERNEL);
s/8/sizeof(stat->stat_id)/
> + if (!p->nvdimm_events_map[count]) {
> rc = -ENOMEM;
> goto out_nvdimm_events_map;
> }
>
> - strcpy(statid, stat->stat_id);
> - p->nvdimm_events_map[count] = statid;
> count++;
> }
> p->nvdimm_events_map[count] = NULL;
> --
> 2.31.1
>
--
Cheers
~ Vaibhav
