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.
> On 10-Jun-2016, at 10:41 PM, Jes Sorensen <Jes.Sorensen@xxxxxxxxxx> wrote:
>
> Nikhil Kshirsagar <nkshirsa@xxxxxxxxxx> writes:
>> 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,
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
