mdadm --detail --scan causes SIGABRT
PATCH 1/1 mdadm --detail --scan causes SIGABRT
Please find attached a patch to fix BZ 1343809.
Details:
mdadm has a buffer overflow if mdinfo->sys_name needs to store a name larger than 20 characters.
Core was generated by `mdadm --detail /dev/md0'.
Program terminated with signal 6, Aborted.
#0 0x0000003a93e325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
(gdb) where
#0 0x0000003a93e325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x0000003a93e33dc5 in abort () at abort.c:92
#2 0x0000003a93e704f7 in __libc_message (do_abort=2, fmt=0x3a93f578cf "*** %s ***: %s terminated\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:198
#3 0x0000003a93f026d7 in __fortify_fail (msg=0x3a93f57875 "buffer overflow detected") at fortify_fail.c:32
#4 0x0000003a93f005c0 in __chk_fail () at chk_fail.c:29
#5 0x000000000044fe59 in strcpy (fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at /usr/include/bits/string3.h:105
#6 sysfs_read (fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at sysfs.c:272
#7 0x000000000041cdfa in Detail (dev=0x7fffe35f1473 "/dev/md0", c=0x7fffe35ef590) at Detail.c:106
#8 0x0000000000405ed3 in misc_list (argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1747
#9 main (argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1425
(gdb)
The line that causes the fault is "sysfs.c" line 272
strcpy(dev->sys_name, de->d_name);
(gdb) print *de
$9 = {d_ino = 14458, d_off = 14471, d_reclen = 40, d_type = 4 '\004',
d_name =
"dev-oczpcie_23_0_ssd\000\207\070\000\000\000\000\000\000\264\070\000\000\000\000\000\000(\000\004dev-oczpcie_11_0_ssd\000\264\070\000\000\000\000\000\000\265\070\000\000\000\000\000\000 \000\bsync_action\000\b\265\070\000\000\000\000\000\000\266\070\000\000\000\000\000\000(\000\blast_sync_action\000\000\000\000\b\266\070\000\000\000\000\000\000\267\070\000\000\000\000\000\000 \000\bmismatch_cnt\000\267\070\000\000\000\000\000\000\270\070\000\000\000\000\000\000(\000\bsync_speed_min\000\000\000\000\000\000\b\270\070\000\000\000\000\000\000\271\070\000\000\000\000\000\000(\000\bsync_speed_max\000\000\000\000\000\000\b\271\070\000\000\000\000\000\000\272\070"} (gdb)
dev-oczpcie_23_0_ssd itself is 20 bytes.
There is no place left for the terminating \0,
(gdb) ptype dev
type = struct mdinfo {
mdu_array_info_t array;
mdu_disk_info_t disk;
__u64 events;
int uuid[4];
char name[33];
long long unsigned int data_offset;
long long unsigned int new_data_offset;
long long unsigned int component_size;
long long unsigned int custom_array_size;
int reshape_active;
long long unsigned int reshape_progress;
int recovery_blocked;
long long unsigned int space_before;
long long unsigned int space_after;
union {
long long unsigned int resync_start;
long long unsigned int recovery_start;
};
long int bitmap_offset;
long unsigned int safe_mode_delay;
int new_level;
int delta_disks;
int new_layout;
int new_chunk;
int errors;
long unsigned int cache_size;
int mismatch_cnt;
char text_version[50];
int container_member;
int container_enough;
char sys_name[20]; <--- 20 .
struct mdinfo *devs;
struct mdinfo *next;
int recovery_fd;
int state_fd;
int prev_state;
int curr_state;
int next_state;
} *(gdb)
The patch increases the size of sys_name[] to 32 bytes to match the size of other device name arrays in the mdadm codebase. A customer reported this issue in SFDC case 01621749.
Thanks,
nikhil.
From 2c3b5692f8c5933e8746305f589efa4edcc00f3c Mon Sep 17 00:00:00 2001
From: Nikhil Kshirsagar <[email protected]>
Date: Fri, 10 Jun 2016 08:50:10 +0530
Subject: [PATCH] Fix for bz 1343809.
The sys_name array in the mdinfo structure is 20 bytes of storage. Increasing the size of this array to 32 bytes.
--- mdadm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mdadm.h +1 -1
@@ -235,7 +235,7 @@ struct mdinfo {int container_enough;/* flag external handlers can set to * indicate that subarrays have not enough (-1), * enough to start (0), or all expected disks (1) */ -char sys_name[20 ]; +char sys_name[32 ];struct mdinfo *devs;struct mdinfo *next;
Signature · 1 line
1.8.3.1
I forgot to add the 'signed off by' part, request the maintainer to add it during the merge.
Thanks,
Nikhil.
Quoted 105 lines · Nikhil Kshirsagar
Please find attached a patch to fix BZ 1343809.
Details:
mdadm has a buffer overflow ifmdinfo->sys_nameneeds to store a name larger than 20 characters.Core was generated by `mdadm --detail
/dev/md0'.
Program terminated with signal 6, Aborted.
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 returnINLINE_SYSCALL(tgkill, 3, pid, selftid, sig);
(gdb) where
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
#10x0000003a93e33dc5in abort () at abort.c:92
#20x0000003a93e704f7in__libc_message(do_abort=2, fmt=0x3a93f578cf "*** %s ***: %s terminated\n") at../sysdeps/unix/sysv/linux/libc_fatal.c:198
#30x0000003a93f026d7in__fortify_fail(msg=0x3a93f57875 "buffer overflow detected") at fortify_fail.c:32
#40x0000003a93f005c0in__chk_fail() at chk_fail.c:29
#50x000000000044fe59in strcpy (fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at/usr/include/bits/string3.h:105
#6sysfs_read(fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at sysfs.c:272
#70x000000000041cdfain Detail (dev=0x7fffe35f1473 "/dev/md0", c=0x7fffe35ef590) at Detail.c:106
#80x0000000000405ed3inmisc_list(argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1747
#9 main (argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1425
(gdb)The line that causes the fault is "
sysfs.c" line 272strcpy(dev->sys_name, de->d_name);(gdb) print *de
$9 = {d_ino= 14458,d_off= 14471,d_reclen= 40,d_type= 4 '\004',d_name=
"dev-oczpcie_23_0_ssd\000\207\070\000\000\000\000\000\000\264\070\000\000\000\000\000\000(\000\004dev-oczpcie_11_0_ssd\000\264\070\000\000\000\000\000\000\265\070\000\000\000\000\000\000 \000\bsync_action\000\b\265\070\000\000\000\000\000\000\266\070\000\000\000\000\000\000(\000\blast_sync_action\000\000\000\000\b\266\070\000\000\000\000\000\000\267\070\000\000\000\000\000\000 \000\bmismatch_cnt\000\267\070\000\000\000\000\000\000\270\070\000\000\000\000\000\000(\000\bsync_speed_min\000\000\000\000\000\000\b\270\070\000\000\000\000\000\000\271\070\000\000\000\000\000\000(\000\bsync_speed_max\000\000\000\000\000\000\b\271\070\000\000\000\000\000\000\272\070"} (gdb)dev-oczpcie_23_0_ssd itself is 20 bytes.
There is no place left for the terminating \0,
(gdb) ptype dev
type = struct mdinfo {
mdu_array_info_tarray;
mdu_disk_info_tdisk;
__u64events;
int uuid[4];
char name[33];
long long unsigned intdata_offset;
long long unsigned intnew_data_offset;
long long unsigned intcomponent_size;
long long unsigned intcustom_array_size;
intreshape_active;
long long unsigned intreshape_progress;
intrecovery_blocked;
long long unsigned intspace_before;
long long unsigned intspace_after;union { long long unsigned int resync_start; long long unsigned int recovery_start;};
long intbitmap_offset;
long unsigned intsafe_mode_delay;
intnew_level;
intdelta_disks;
intnew_layout;
intnew_chunk;
int errors;
long unsigned intcache_size;
intmismatch_cnt;
char text_version[50];
intcontainer_member;
intcontainer_enough;
char sys_name[20]; <--- 20 .
struct mdinfo *devs;
struct mdinfo *next;
intrecovery_fd;
intstate_fd;
intprev_state;
intcurr_state;
intnext_state;
} *
(gdb)The patch increases the size of sys_name[] to 32 bytes to match the size of other device name arrays in the mdadm codebase. A customer reported this issue in SFDC case 01621749.
Thanks,
nikhil.<0001-Fix-for-bz-1343809.patch>
Quoted 47 lines · Nikhil Kshirsagar
Please find attached a patch to fix BZ 1343809.
Details:
mdadm has a buffer overflow ifmdinfo->sys_nameneeds to store a name larger than 20 characters.Core was generated by `mdadm --detail
/dev/md0'.
Program terminated with signal 6, Aborted.
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 returnINLINE_SYSCALL(tgkill, 3, pid, selftid, sig);
(gdb) where
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
#10x0000003a93e33dc5in abort () at abort.c:92
#20x0000003a93e704f7in__libc_message(do_abort=2, fmt=0x3a93f578cf "*** %s ***: %s terminated\n") at../sysdeps/unix/sysv/linux/libc_fatal.c:198
#30x0000003a93f026d7in__fortify_fail(msg=0x3a93f57875 "buffer overflow detected") at fortify_fail.c:32
#40x0000003a93f005c0in__chk_fail() at chk_fail.c:29
#50x000000000044fe59in strcpy (fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at/usr/include/bits/string3.h:105
#6sysfs_read(fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at sysfs.c:272
#70x000000000041cdfain Detail (dev=0x7fffe35f1473 "/dev/md0", c=0x7fffe35ef590) at Detail.c:106
#80x0000000000405ed3inmisc_list(argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1747
#9 main (argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1425
(gdb)The line that causes the fault is "
sysfs.c" line 272strcpy(dev->sys_name, de->d_name);(gdb) print *de
$9 = {d_ino= 14458,d_off= 14471,d_reclen= 40,d_type= 4 '\004',
d_name=
"dev-oczpcie_23_0_ssd\000\207\070\000\000\000\000\000\000\264\070\000\000\000\000\000\000(\000\004dev-oczpcie_11_0_ssd\000\264\070\000\000\000\000\000\000\265\070\000\000\000\000\000\000 \000\bsync_action\000\b\265\070\000\000\000\000\000\000\266\070\000\000\000\000\000\000(\000\blast_sync_action\000\000\000\000\b\266\070\000\000\000\000\000\000\267\070\000\000\000\000\000\000 \000\bmismatch_cnt\000\267\070\000\000\000\000\000\000\270\070\000\000\000\000\000\000(\000\bsync_speed_min\000\000\000\000\000\000\b\270\070\000\000\000\000\000\000\271\070\000\000\000\000\000\000(\000\bsync_speed_max\000\000\000\000\000\000\b\271\070\000\000\000\000\000\000\272\070"} (gdb)
Quoted 3 lines
dev-oczpcie_23_0_ssd itself is 20 bytes.
There is no place left for the terminating \0,
Nikhil,
I am curious, how did you get that long device name? I tried to reproduce the problem here using /dev/disk/by-id/ devices, but they get converted to the shorter /dev/sdX names automatically so it doesn't trigger.
I do not disagree we need to fix this, but I am curious of the real life scenario how you hit the problem?
Right now I am more wondering whether we should handle this in a more generic way by allocating an appriopriately sized buffer or bump up the name buffer the way your patch did it.
Cheers,
Jes
Hi Jes ,
Would it help to examine the core file ? It's present on the machine and location specified in the bz comments . That's how I saw the data structure that had the issue . Indeed there are other device names where the name does not overflow since they are 32 bytes (which is why I chose this value) or where the name *does* get truncated. However this truncation does not seem to happen for de->d_name which is then copied into dev->sys_name.
As for allocating an appropriate size on the heap instead of a static array it does make sense and I can correct the fix to do that but there are lots of other device names which are static arrays. So which ones do we change ?
-nikhil.
Quoted 70 lines · Jes Sorensen
Quoted 47 lines · Nikhil Kshirsagar
Please find attached a patch to fix BZ 1343809.
Details:
mdadm has a buffer overflow ifmdinfo->sys_nameneeds to store a name larger than 20 characters.Core was generated by `mdadm --detail
/dev/md0'.
Program terminated with signal 6, Aborted.
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 returnINLINE_SYSCALL(tgkill, 3, pid, selftid, sig);
(gdb) where
#00x0000003a93e325e5in raise (sig=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:64
#10x0000003a93e33dc5in abort () at abort.c:92
#20x0000003a93e704f7in__libc_message(do_abort=2, fmt=0x3a93f578cf "*** %s ***: %s terminated\n") at../sysdeps/unix/sysv/linux/libc_fatal.c:198
#30x0000003a93f026d7in__fortify_fail(msg=0x3a93f57875 "buffer overflow detected") at fortify_fail.c:32
#40x0000003a93f005c0in__chk_fail() at chk_fail.c:29
#50x000000000044fe59in strcpy (fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at/usr/include/bits/string3.h:105
#6sysfs_read(fd=<value optimized out>, devnm=<value optimized out>, options=<value optimized out>) at sysfs.c:272
#70x000000000041cdfain Detail (dev=0x7fffe35f1473 "/dev/md0", c=0x7fffe35ef590) at Detail.c:106
#80x0000000000405ed3inmisc_list(argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1747
#9 main (argc=<value optimized out>, argv=<value optimized out>) at mdadm.c:1425
(gdb)The line that causes the fault is "
sysfs.c" line 272strcpy(dev->sys_name, de->d_name);(gdb) print *de
$9 = {d_ino= 14458,d_off= 14471,d_reclen= 40,d_type= 4 '\004',
d_name=
"dev-oczpcie_23_0_ssd\000\207\070\000\000\000\000\000\000\264\070\000\000\000\000\000\000(\000\004dev-oczpcie_11_0_ssd\000\264\070\000\000\000\000\000\000\265\070\000\000\000\000\000\000 \000\bsync_action\000\b\265\070\000\000\000\000\000\000\266\070\000\000\000\000\000\000(\000\blast_sync_action\000\000\000\000\b\266\070\000\000\000\000\000\000\267\070\000\000\000\000\000\000 \000\bmismatch_cnt\000\267\070\000\000\000\000\000\000\270\070\000\000\000\000\000\000(\000\bsync_speed_min\000\000\000\000\000\000\b\270\070\000\000\000\000\000\000\271\070\000\000\000\000\000\000(\000\bsync_speed_max\000\000\000\000\000\000\b\271\070\000\000\000\000\000\000\272\070"} (gdb)Quoted 3 lines
dev-oczpcie_23_0_ssd itself is 20 bytes.
There is no place left for the terminating \0,
Nikhil,
I am curious, how did you get that long device name? I tried to reproduce the problem here using
/dev/disk/by-id/devices, but they get converted to the shorter/dev/sdXnames automatically so it doesn't trigger.I do not disagree we need to fix this, but I am curious of the real life scenario how you hit the problem?
Right now I am more wondering whether we should handle this in a more generic way by allocating an appriopriately sized buffer or bump up the name buffer the way your patch did it.
Cheers,
Jes
Quoted 10 lines · Nikhil Kshirsagar
Hi Jes ,
Would it help to examine the core file ? It's present on the machine and location specified in the bz comments . That's how I saw the data structure that had the issue . Indeed there are other device names where the name does not overflow since they are 32 bytes (which is why I chose this value) or where the name *does* get truncated. However this truncation does not seem to happen for
de->d_namewhich is then copied intodev->sys_name.
Quoted 4 lines
As for allocating an appropriate size on the heap instead of a static array it does make sense and I can correct the fix to do that but there are lots of other device names which are static arrays. So which ones do we change ?
This is the tricky part, sys_name is used in a lot of places in different ways.
Do you know if they have a /dev/oczpcie_11_0_ssd on the system, and if they do, how did that device get created?
Cheers,
Jes
I think the sosreport should contain the info of the raid devices even though mdadm crashes so we don't see that output in there , there is proc/mdstat and other raid related info there. Let me check.
If nothing else I can ask them for proc/mdstat and mdadm -detail again with the working test binary i sent them.
-Nikhil.
Quoted 26 lines · Jes Sorensen
Quoted 10 lines · Nikhil Kshirsagar
Hi Jes ,
Would it help to examine the core file ? It's present on the machine and location specified in the bz comments . That's how I saw the data structure that had the issue . Indeed there are other device names where the name does not overflow since they are 32 bytes (which is why I chose this value) or where the name *does* get truncated. However this truncation does not seem to happen for
de->d_namewhich is then copied intodev->sys_name.Quoted 4 lines
As for allocating an appropriate size on the heap instead of a static array it does make sense and I can correct the fix to do that but there are lots of other device names which are static arrays. So which ones do we change ?
This is the tricky part,
sys_nameis used in a lot of places in different ways.Do you know if they have a
/dev/oczpcie_11_0_ssdon the system, and if they do, how did that device get created?Cheers,
Jes
Hi Jes,
yes those devices exist,
/dev/oczpcie_11_0_ssd: using cached size 6251381753 sectors
The devices were created by the OCZ driver.
Thanks,
Nikhil.
Quoted 32 lines · Nikhil Kshirsagar
I think the sosreport should contain the info of the raid devices even though mdadm crashes so we don't see that output in there , there is proc/mdstat and other raid related info there. Let me check.
If nothing else I can ask them for proc/mdstat and mdadm -detail again with the working test binary i sent them.
-Nikhil.
Quoted 25 lines · Jes Sorensen
Quoted 10 lines · Nikhil Kshirsagar
Hi Jes ,
Would it help to examine the core file ? It's present on the machine and location specified in the bz comments . That's how I saw the data structure that had the issue . Indeed there are other device names where the name does not overflow since they are 32 bytes (which is why I chose this value) or where the name *does* get truncated. However this truncation does not seem to happen for
de->d_namewhich is then copied intodev->sys_name.Quoted 4 lines
As for allocating an appropriate size on the heap instead of a static array it does make sense and I can correct the fix to do that but there are lots of other device names which are static arrays. So which ones do we change ?
This is the tricky part,
sys_nameis used in a lot of places in different ways.Do you know if they have a
/dev/oczpcie_11_0_ssdon the system, and if they do, how did that device get created?Cheers,
Jes
Quoted 5 lines · Nikhil Kshirsagar
Please find attached a patch to fix BZ 1343809.
Details:
mdadm has a buffer overflow ifmdinfo->sys_nameneeds to store a name larger than 20 characters.
Nikhil,
I have applied this one, adding the Signed-off-by: line as you requested and slightly adjusted the patch description.
I would like to include some checking fixes too to avoid us overflowing the new buffer size with even longer names.
Cheers,
Jes