Skip to content

Commit 3a774a1

Browse files
Etsukatagregkh
authored andcommitted
EDAC: Fix global-out-of-bounds write when setting edac_mc_poll_msec
[ Upstream commit d8655e7 ] Commit 9da21b1 ("EDAC: Poll timeout cannot be zero, p2") assumes edac_mc_poll_msec to be unsigned long, but the type of the variable still remained as int. Setting edac_mc_poll_msec can trigger out-of-bounds write. Reproducer: # echo 1001 > /sys/module/edac_core/parameters/edac_mc_poll_msec KASAN report: BUG: KASAN: global-out-of-bounds in edac_set_poll_msec+0x140/0x150 Write of size 8 at addr ffffffffb91b2d00 by task bash/1996 CPU: 1 PID: 1996 Comm: bash Not tainted 5.2.0-rc6+ #23 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-2.fc30 04/01/2014 Call Trace: dump_stack+0xca/0x13e print_address_description.cold+0x5/0x246 __kasan_report.cold+0x75/0x9a ? edac_set_poll_msec+0x140/0x150 kasan_report+0xe/0x20 edac_set_poll_msec+0x140/0x150 ? dimmdev_location_show+0x30/0x30 ? vfs_lock_file+0xe0/0xe0 ? _raw_spin_lock+0x87/0xe0 param_attr_store+0x1b5/0x310 ? param_array_set+0x4f0/0x4f0 module_attr_store+0x58/0x80 ? module_attr_show+0x80/0x80 sysfs_kf_write+0x13d/0x1a0 kernfs_fop_write+0x2bc/0x460 ? sysfs_kf_bin_read+0x270/0x270 ? kernfs_notify+0x1f0/0x1f0 __vfs_write+0x81/0x100 vfs_write+0x1e1/0x560 ksys_write+0x126/0x250 ? __ia32_sys_read+0xb0/0xb0 ? do_syscall_64+0x1f/0x390 do_syscall_64+0xc1/0x390 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x7fa7caa5e970 Code: 73 01 c3 48 8b 0d 28 d5 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 99 2d 2c 00 00 75 10 b8 01 00 00 00 04 RSP: 002b:00007fff6acfdfe8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007fa7caa5e970 RDX: 0000000000000005 RSI: 0000000000e95c08 RDI: 0000000000000001 RBP: 0000000000e95c08 R08: 00007fa7cad1e760 R09: 00007fa7cb36a700 R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000005 R13: 0000000000000001 R14: 00007fa7cad1d600 R15: 0000000000000005 The buggy address belongs to the variable: edac_mc_poll_msec+0x0/0x40 Memory state around the buggy address: ffffffffb91b2c00: 00 00 00 00 fa fa fa fa 00 00 00 00 fa fa fa fa ffffffffb91b2c80: 00 00 00 00 fa fa fa fa 00 00 00 00 fa fa fa fa >ffffffffb91b2d00: 04 fa fa fa fa fa fa fa 04 fa fa fa fa fa fa fa ^ ffffffffb91b2d80: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 ffffffffb91b2e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Fix it by changing the type of edac_mc_poll_msec to unsigned int. The reason why this patch adopts unsigned int rather than unsigned long is msecs_to_jiffies() assumes arg to be unsigned int. We can avoid integer conversion bugs and unsigned int will be large enough for edac_mc_poll_msec. Reviewed-by: James Morse <james.morse@arm.com> Fixes: 9da21b1 ("EDAC: Poll timeout cannot be zero, p2") Signed-off-by: Eiichi Tsukata <devel@etsukata.com> Signed-off-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 0ecba23 commit 3a774a1

File tree

2 files changed

+9
-9
lines changed

2 files changed

+9
-9
lines changed

drivers/edac/edac_mc_sysfs.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
static int edac_mc_log_ue = 1;
2727
static int edac_mc_log_ce = 1;
2828
static int edac_mc_panic_on_ue;
29-
static int edac_mc_poll_msec = 1000;
29+
static unsigned int edac_mc_poll_msec = 1000;
3030

3131
/* Getter functions for above */
3232
int edac_mc_get_log_ue(void)
@@ -45,30 +45,30 @@ int edac_mc_get_panic_on_ue(void)
4545
}
4646

4747
/* this is temporary */
48-
int edac_mc_get_poll_msec(void)
48+
unsigned int edac_mc_get_poll_msec(void)
4949
{
5050
return edac_mc_poll_msec;
5151
}
5252

5353
static int edac_set_poll_msec(const char *val, const struct kernel_param *kp)
5454
{
55-
unsigned long l;
55+
unsigned int i;
5656
int ret;
5757

5858
if (!val)
5959
return -EINVAL;
6060

61-
ret = kstrtoul(val, 0, &l);
61+
ret = kstrtouint(val, 0, &i);
6262
if (ret)
6363
return ret;
6464

65-
if (l < 1000)
65+
if (i < 1000)
6666
return -EINVAL;
6767

68-
*((unsigned long *)kp->arg) = l;
68+
*((unsigned int *)kp->arg) = i;
6969

7070
/* notify edac_mc engine to reset the poll period */
71-
edac_mc_reset_delay_period(l);
71+
edac_mc_reset_delay_period(i);
7272

7373
return 0;
7474
}
@@ -82,7 +82,7 @@ MODULE_PARM_DESC(edac_mc_log_ue,
8282
module_param(edac_mc_log_ce, int, 0644);
8383
MODULE_PARM_DESC(edac_mc_log_ce,
8484
"Log correctable error to console: 0=off 1=on");
85-
module_param_call(edac_mc_poll_msec, edac_set_poll_msec, param_get_int,
85+
module_param_call(edac_mc_poll_msec, edac_set_poll_msec, param_get_uint,
8686
&edac_mc_poll_msec, 0644);
8787
MODULE_PARM_DESC(edac_mc_poll_msec, "Polling period in milliseconds");
8888

drivers/edac/edac_module.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ extern int edac_mc_get_log_ue(void);
3636
extern int edac_mc_get_log_ce(void);
3737
extern int edac_mc_get_panic_on_ue(void);
3838
extern int edac_get_poll_msec(void);
39-
extern int edac_mc_get_poll_msec(void);
39+
extern unsigned int edac_mc_get_poll_msec(void);
4040

4141
unsigned edac_dimm_info_location(struct dimm_info *dimm, char *buf,
4242
unsigned len);

0 commit comments

Comments
 (0)