Re: [PATCH] powerpc/papr_scm: Fix buffer overflow issue with CONFIG_FORTIFY_SOURCE
On Thu, 5 May 2022 21:04:51 +0530, Kajol Jain wrote: > 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. > > [...] Applied to powerpc/fixes. [1/1] powerpc/papr_scm: Fix buffer overflow issue with CONFIG_FORTIFY_SOURCE https://git.kernel.org/powerpc/c/348c71344111d7a48892e3e52264ff11956fc196 cheers
Re: [PATCH] powerpc/papr_scm: Fix buffer overflow issue with CONFIG_FORTIFY_SOURCE
Hi Kajol, Thanks for the patch. Minor review comment below: Kajol Jain 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 [c077dad0] fortify_panic+0x28/0x38 > [0.090382][T1] LR [c077dacc] fortify_panic+0x24/0x38 > [0.090387][T1] Call Trace: > [0.090390][T1] [c022d77836e0] [c077dacc] > fortify_panic+0x24/0x38 (unreliable) > [9.297707] [T1] [c0080deb2660] > papr_scm_pmu_check_events.constprop.0+0x118/0x220 [papr_scm] > [9.297721] [T1] [c0080deb2cb0] papr_scm_probe+0x288/0x62c > [papr_scm] > [9.297732] [T1] [c09b46a8] 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 > --- > 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
[PATCH] powerpc/papr_scm: Fix buffer overflow issue with CONFIG_FORTIFY_SOURCE
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 [c077dad0] fortify_panic+0x28/0x38 [0.090382][T1] LR [c077dacc] fortify_panic+0x24/0x38 [0.090387][T1] Call Trace: [0.090390][T1] [c022d77836e0] [c077dacc] fortify_panic+0x24/0x38 (unreliable) [9.297707] [T1] [c0080deb2660] papr_scm_pmu_check_events.constprop.0+0x118/0x220 [papr_scm] [9.297721] [T1] [c0080deb2cb0] papr_scm_probe+0x288/0x62c [papr_scm] [9.297732] [T1] [c09b46a8] 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 --- 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); + 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