[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
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
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
