Bug 208333 - Kst kills itself with a SIGABRT in the meminfo() function
Summary: Kst kills itself with a SIGABRT in the meminfo() function
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.8.0
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-23 23:08 UTC by Andrew Walker
Modified: 2009-09-29 03:31 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Walker 2009-09-23 23:08:46 UTC
Version:           1.8.0 (using KDE 4.3.0)
OS:                Linux
Installed from:    Compiled From Sources

The following is a bug report from Michael:

=======

I'm running kst-1.8.0-3.fc12.i686.rpm recompiled for FC10 on Fedora
10. The same problem also occurred with kst-1.7.0-3.fc10.i386.rpm.

After running for a few hours, Kst kills itself with a SIGABRT in the
meminfo() function. Here's a backtrace from gdb:

#0  0x0028f424 in __kernel_vsyscall ()
#1  0x00e0b460 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x00e0ce28 in abort () at abort.c:88
#3  0x00e48fed in __libc_message (do_abort=2,
    fmt=0xf2211c "*** %s ***: %s terminated\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:170
#4  0x00edd938 in __fortify_fail (msg=0xf220c6 "buffer overflow detected")
    at fortify_fail.c:32
#5  0x00edba30 in __chk_fail () at chk_fail.c:29
#6  0x00edacf4 in __strcpy_chk (dest=0xbfffe248 "PageTables:     ",
    src=0x60a808 "PageTables", destlen=11825) at strcpy_chk.c:61
#7  0x005c9356 in strcpy () at /usr/include/bits/string3.h:106
#8  meminfo () at sysinfo.c:537
#9  0x03735ef9 in KstApp::updateMemoryStatus (this=0x80ccff0) at kst.cpp:2699
...

glibc thinks that the strcpy() overflowed its destination buffer, but
I don't think it actually has. The __fortify_fail() call is part of a
buffer overflow protection scheme in glibc that was introduced for C++
in Fedora 8. See the FORTIFY_SOURCE section here:
http://fedoraproject.org/wiki/Security/Features

#8  meminfo () at sysinfo.c:537
537     strcpy(namebuf,head);

(gdb) p namebuf
$6 = "PageTables:     "
(gdb) p head
$7 = 0x60a808 "PageTables"
Comment 1 Michael Vincent 2009-09-24 17:43:30 UTC
I believe that the reason it failed is because destlen in __strcpy_chk() is much bigger than the size of dest (16 bytes):

__strcpy_chk (dest=0xbfffe248 "PageTables:     ", src=0x60a808 "PageTables", destlen=11825)

I don't know if an incorrect value was passed-in by strcpy():

__builtin___strcpy_chk (__dest, __src, __bos (__dest))

or if destlen underflowed and wrapped around, since it is a size_t and __strcpy_chk() decrements it.

I've been running Kst overnight now without any problems, so this problem appears to be intermittent and may be dependent on the specific machine, kernel, or glibc and not really a kst-specific bug.
Comment 2 Michael Vincent 2009-09-28 22:22:42 UTC
This strcpy() failure happened again over the weekend.

----------------------------------------------------------------------
__extern_always_inline char *
__NTH (strcpy (char *__restrict __dest, __const char *__restrict __src))
{
  return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
}
----------------------------------------------------------------------

----------------------------------------------------------------------
/* Copy SRC to DEST with checking of destination buffer overflow.  */
char *
__strcpy_chk (dest, src, destlen)
     char *dest;
     const char *src;
     size_t destlen;
{
  reg_char c;
  char *s = (char *) src;
  const ptrdiff_t off = dest - s;

  while (__builtin_expect (destlen >= 4, 0))
    {
      c = s[0];
      s[off] = c;
      if (c == '\0')
        return dest;
      c = s[1];
      s[off + 1] = c;
      if (c == '\0')
        return dest;
      c = s[2];
      s[off + 2] = c;
      if (c == '\0')
        return dest;
      c = s[3];
      s[off + 3] = c;
      if (c == '\0')
        return dest;
      destlen -= 4;
      s += 4;
    }

// ----------------- breakpoint here

  do
    {
      if (__builtin_expect (destlen-- == 0, 0))
        __chk_fail ();
      c = *s;
      *(s++ + off) = c;
    }
  while (c != '\0');

  return dest;
}
----------------------------------------------------------------------

__bos() is macro'ed to __builtin_object_size() that is supposed to return the size of __dest.

When gdb stops at the specified breakpoint, there are less than 4 bytes left in dest. In this case destlen is 0 at the breakpoint – probably because dest is a 16 byte char array (multiple of 4). Since destlen is passed in on the stack and modified in place and since the return value of __bos() is never assigned to a variable, I don't think that there's any direct way of printing out the initial value of destlen. However, we can use the difference between src and s to determine how many times the while() loop iterated and therefore the initial value of destlen.

src = 0x60a968
s = 0x60a978
s - src = 16

So it looks like __bos() returned the correct size of dest. That means that src must be too big.

(gdb) x /20bc src
0x60a968 <buf+872>:	68 'D'	105 'i'	114 'r'	101 'e'	99 'c'	116 't'	77 'M'	97 'a'
0x60a970 <buf+880>:	112 'p'	52 '4'	107 'k'	0 '\0'	32 ' '	32 ' '	32 ' '	32 ' '
0x60a978 <buf+888>:	55 '7'	55 '7'	48 '0'	48 '0'

Well, src is correctly NULL terminated at the 12th character. The function should have returned on the fourth character of the third iteration through the while() loop. What's weird is that dest isn't NULL terminated until several characters later – probably from a previous call to this function. It has a : in place of the \0 and the following four characters are the same as src.

(gdb) x /20bc dest
0xbfffe248:	68 'D'	105 'i'	114 'r'	101 'e'	99 'c'	116 't'	77 'M'	97 'a'
0xbfffe250:	112 'p'	52 '4'	107 'k'	58 ':'	32 ' '	32 ' '	32 ' '	32 ' '
0xbfffe258:	0 '\0'	-4 '�'	-21 '�'	0 '\0'

So it seems that a : was read instead of a \0 which causes the function to attempt to overflow the destination. Could another thread be calling meminfo() at the same time? meminfo() does get called from a few different places. I'm not sure if any of the calls are made from different threads though. The strcpy() that fails is in meminfo():

static char buf[1024];
...
char *head;
...
head = buf;
...
strcpy(namebuf,head);


So, head points to somewhere in a statically allocated buffer. That doesn't look thread-safe to me!
Comment 3 Andrew Walker 2009-09-29 00:15:53 UTC
Many thanks for the info.

meminfo() is indeed called from two different threads (the main thread, and the data update thread), which obviously it never should have done given that meminfo is not thread safe.

I'll checkin a fix shortly.
Comment 4 Andrew Walker 2009-09-29 03:31:24 UTC
SVN commit 1029126 by arwalker:

BUG:208333 make all access to meminfo thread safe by entry through a single mutex

 M  +33 -7     libkst/kstdatacollection.cpp  
 M  +1 -0      libkst/kstdatacollection.h  
 M  +13 -12    libkstapp/kst.cpp  
 M  +1 -2      libkstapp/kstdatawizard_i.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1029126