| Message ID | 20220505153451.35503-1-kjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 348c71344111d7a48892e3e52264ff11956fc196 |
| Headers | show |
| Series | powerpc/papr_scm: Fix buffer overflow issue with CONFIG_FORTIFY_SOURCE | expand |
Hi Kajol, Thanks for the patch. Minor review comment below: Kajol Jain <kjain@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 <kjain@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 >
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
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;
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 <kjain@linux.ibm.com> --- arch/powerpc/platforms/pseries/papr_scm.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)