Summary: | Kst kills itself with a SIGABRT in the meminfo() function | ||
---|---|---|---|
Product: | [Applications] kst | Reporter: | Andrew Walker <arwalker> |
Component: | general | Assignee: | kst |
Status: | RESOLVED FIXED | ||
Severity: | crash | CC: | bug.zilla.vynce |
Priority: | NOR | ||
Version: | 1.8.0 | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: |
Description
Andrew Walker
2009-09-23 23:08:46 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. 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! 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. 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 |