10:25 a.m.
Shared library are heavily used through Linux distributions. Unfortunately, there are
cases of functions in the libraries having undefined behavior. Rather than immediately
reporting the dependence on that undefined behavior, the applications may later fail in
odd and seemingly random ways. On particular example of this problem is the memcpy
function which has undefined behavior when the source and destination regions overlap.
This resulted in the following bug being filled about "Strange sound on mp3 flash
website":
http://koji.fedoraproject.org/koji/taskinfo?taskID=2898613
The diagnosis of this problem was not straightford because the memcpy silently corrupted
the data in the copy. There are many other examples of this type of memcpy problems in
bugzilla.
What would be desirable is catching the dependency on undefined behavior when it occurs.
The LD_PRELOAD environment variable allows wrappers for shared library functions to be
inserted. These wrappers can do additional checks and flag those issues when they occur.
The mutrace package in Fedora is one example of this approach. It makes use of this
mechanism to instrument the mutex operations and can trigger a gdb breakpoint when a
problem mutex operation occurs.
I have taken the code in the mutrace package and made memstomp which looks for the memcpy
of overlapping regions.
git repo at:
http://fedorapeople.org/gitweb?p=wcohen/public_git/memstomp;a=summary
A fedora scratch package RPM at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2898613
Valgrind does check the arguments for memcpy (and many other memory related checks). The
main advantage to using the specialized wrappers like memstomp is lower overhead. Most
people are not willing to pay for the overhead that valgrind introduces (4x-100x slow
downs). The overhead for the memstomp wrappers should be low enough that it would be
feasible to set the LD_PRELOAD for Fedora alpha releases. This would make the problems
depending on undefined behavior obvious rather than spending a large amount of time trying
to replicate the problem and then diagnosing it.
-Will
10:40 a.m.
On Thu, Mar 10, 2011 at 11:25:38AM -0500, William Cohen wrote:
git repo at:
http://fedorapeople.org/gitweb?p=wcohen/public_git/memstomp;a=summary
A fedora scratch package RPM at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2898613
Valgrind does check the arguments for memcpy (and many other memory
related checks). The main advantage to using the specialized wrappers
like memstomp is lower overhead. Most people are not willing to pay
for the overhead that valgrind introduces (4x-100x slow downs). The
overhead for the memstomp wrappers should be low enough that it would
be feasible to set the LD_PRELOAD for Fedora alpha releases. This
would make the problems depending on undefined behavior obvious rather
than spending a large amount of time trying to replicate the problem
and then diagnosing it.
There could be problems with the memstomp PRELOAD wrapper function
for any application code which is using memcpy() from within a signal
handler. Functions like fprintf() & malloc() are not async signal
safe, so could deadlock the process or worse. The odds of hitting this
problem are fairly low, but wanted to point out that speed isn't the
only concern wrt enabling a global distro LD_PRELOAD like this.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:05 a.m.
On Thu, Mar 10, 2011 at 11:25:38AM -0500, William Cohen wrote:
Shared library are heavily used through Linux distributions.
Unfortunately, there are cases of functions in the libraries having
undefined behavior. Rather than immediately reporting the dependence on
that undefined behavior, the applications may later fail in odd and
seemingly random ways. On particular example of this problem is the
memcpy function which has undefined behavior when the source and
destination regions overlap. This resulted in the following bug being
filled about "Strange sound on mp3 flash website":
http://koji.fedoraproject.org/koji/taskinfo?taskID=2898613
The diagnosis of this problem was not straightford because the memcpy
silently corrupted the data in the copy. There are many other examples of
this type of memcpy problems in bugzilla.
What would be desirable is catching the dependency on undefined behavior
when it occurs. The LD_PRELOAD environment variable allows wrappers for
shared library functions to be inserted. These wrappers can do additional
checks and flag those issues when they occur. The mutrace package in
Fedora is one example of this approach. It makes use of this mechanism to
instrument the mutex operations and can trigger a gdb breakpoint when a
problem mutex operation occurs.
I have taken the code in the mutrace package and made memstomp which looks
for the memcpy of overlapping regions.
git repo at:
http://fedorapeople.org/gitweb?p=wcohen/public_git/memstomp;a=summary
A fedora scratch package RPM at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2898613
Valgrind does check the arguments for memcpy (and many other memory
related checks). The main advantage to using the specialized wrappers
like memstomp is lower overhead. Most people are not willing to pay for
the overhead that valgrind introduces (4x-100x slow downs). The overhead
for the memstomp wrappers should be low enough that it would be feasible
to set the LD_PRELOAD for Fedora alpha releases. This would make the
problems depending on undefined behavior obvious rather than spending a
large amount of time trying to replicate the problem and then diagnosing
it.
Nice, but I think the dlsym (NULL, "main") lookup should not be done, at
least not by default, we really don't want to encourage people linking
programs with -rdynamic, that adds a runtime penalty.
And, it would be nice when you have such a library not to check just
memcpy, there are plenty of other commonly used calls which could
be warned about.
memcpy, strcpy, strncpy, strcat, strncat, strtok, strtok_r, mempcpy, strsep,
stpcpy, stpncpy, memccpy
just to name a few from <string.h>, then for -D_FORTIFY_SOURCE also
__memcpy_chk, __mempcpy_chk, __strcpy_chk, __stpcpy_chk, __strncpy_chk,
__stpncpy_chk, __strcat_chk, __strncat_chk.
In wchar.h e.g.
wcscpy, wcsncpy, wcscat, wcsncat, wcstok, wmemcpy, wmempcpy
and maybe
mbrtowc, wcrtomb, mbrlen, mbsrtowcs, wcsrtombs, mbsnrtowcs,
wcsnrtombs, wcstol, wcstoul, wcstoll, wcstoull, ...
Maybe also sprintf/snprintf if format string contains some %s/%ls/%S
specifiers and those arguments overlap the target.
Basically, most of the __restrict/restrict qualified prototypes
in glibc headers would be good candidates for overlap tests (if possible
to determine length).
In the implementation of the checking library you probably want
to #include <sys/cdefs.h>, then #undef __restrict #define __restrict
and similarly for restrict and __restrict_arr and compile the
file with -fno-builtin, to make sure gcc doesn't optimize your checks
away based on the arguments being restricted pointers.
Jakub
1:28 p.m.
New subject: Using LD_PRELOAD wrappers to identify problem use of shared library functions
On 03/10/2011, Jakub Jelinek wrote:
And, it would be nice when you have such a library not to check just
memcpy, there are plenty of other commonly used calls which could
be warned about.
memcpy, strcpy, strncpy, strcat, strncat, strtok, strtok_r, mempcpy, strsep,
stpcpy, stpncpy, memccpy
The vast majority of the benefit of checking for overlap comes from checking
only memcpy, strcat, and strcpy. The usage of other mem* and str* is much less.
[And detecting overlap in routines such as strncpy is not trivial.
I have been working on W.Cohen's memstomp.]
just to name a few from <string.h>, then for -D_FORTIFY_SOURCE
also
__memcpy_chk, __mempcpy_chk, __strcpy_chk, __stpcpy_chk, __strncpy_chk,
__stpncpy_chk, __strcat_chk, __strncat_chk.
Those *_chk routines already perform length checks and have a mechanism for
handling bad usage. Any implementation of *_chk which ignores the possibility
of overlap (does not detect violations of 'restrict') arguably has a bug
in Usability, if not in [the spirit of] Functionality.
In wchar.h e.g.
wcscpy, wcsncpy, wcscat, wcsncat, wcstok, wmemcpy, wmempcpy
and maybe
mbrtowc, wcrtomb, mbrlen, mbsrtowcs, wcsrtombs, mbsnrtowcs,
wcsnrtombs, wcstol, wcstoul, wcstoll, wcstoull, ...
Maybe also sprintf/snprintf if format string contains some %s/%ls/%S
specifiers and those arguments overlap the target.
Basically, most of the __restrict/restrict qualified prototypes
in glibc headers would be good candidates for overlap tests (if possible
to determine length).
For 90% or more of the 'restrict' declarations, it is not worth developing
a separate library with checking code. Those checks should be in the
everyday library already, and active always. For instance, any syscall
wrapper (such as select()) certainly should enforce 'restrict'; the few
cycles to check for overlap are insignificant compared to a syscall.
Not detecting overlap of multiple outputs (select() is an example again)
produces *wrong answers*; this certainly is a bug to be fixed.
For most routines with 'restrict' arguments, the frequency of use
is so small that the total time cost of checking for overlap (for all
such routines *combined*) probably is less than a few seconds per day.
(An example is wcstouq.)
Using a separate library to detect overlap suffers from bad psychology.
The recent episode of memcpy in libflashplayer might account for 1/3 or more
of all the important cases in this year 2011. A project which uses a
checking library likely will find a few overlaps immediately, but after
that only a couple per year. Repeated results of "No bad usage detected"
is a strong encouragement to discontinue "extra" work, especially
anything that requires additional setup or administration.
There is no substitute for detecting overlap all the time and every time.
Such checking likely is less expensive than the cost of *ONE* bug per year,
and therefore the everyday library should do it.
--
9:15 p.m.
On 03/10/2011 08:25 AM, William Cohen wrote:
I have taken the code in the mutrace package and made memstomp which
looks for the memcpy of overlapping regions.
git repo at:
http://fedorapeople.org/gitweb?p=wcohen/public_git/memstomp;a=summary
Did anybody else run into trouble trying to clone the git repository (as per the README)?
$ git clone http://wcohen.fedorapeople.org/git/memstomp/
Cloning into memstomp...
fatal: http://wcohen.fedorapeople.org/git/memstomp/info/refs not found: did you run git
update-server-info on the server?
$
--
11:29 p.m.
On 03/10/2011 08:25 AM, William Cohen wrote:
Actually: git clone git://fedorapeople.org/home/fedora/wcohen/public_git/memstomp
The implementation has some properties:
1. Not async signal safe [malloc, fprintf], as noted previously by Daniel Berrange.
2. Not thread safe: unguarded top-level static variables in backtrace-symbols.c.
3. Essentially bundles a private copy of libbfd.
4. Needs work for a process tree that uses a mixture of 32-bit and 64-bit programs.
5. Does not catch violations in compile-time inlined expansions.
6. SIGSEGVs for violations from just-in-time compiled code: uninit local
variables in backtrace_symbols() not set by calls to dl_iterate_phdr.
--
9:33 a.m.
New subject: Using LD_PRELOAD wrappers to identify problem use of shared library functions
On 03/11/2011 12:29 AM, John Reiser wrote:
On 03/10/2011 08:25 AM, William Cohen wrote:
> git repo at:
>
> http://fedorapeople.org/gitweb?p=wcohen/public_git/memstomp;a=summary
Actually: git clone git://fedorapeople.org/home/fedora/wcohen/public_git/memstomp
The implementation has some properties:
1. Not async signal safe [malloc, fprintf], as noted previously by Daniel Berrange.
2. Not thread safe: unguarded top-level static variables in backtrace-symbols.c.
3. Essentially bundles a private copy of libbfd.
backtrace code was copied from mutrace to get the memstomp proof-of-concept working. It
would be preferable to not have static libbfd libraries. Improved backtrace code would
address a number of the issues mentioned here (2, 3, and 6).
4. Needs work for a process tree that uses a mixture of 32-bit and
64-bit programs.
In theory, if both the 32-bit and 64-bit versions of the shared libraries are installed in
the usual directories, the ldconfig has been run, and absolute paths are not used in
LD_PRELOAD then the loader should find the correct version of the library.
5. Does not catch violations in compile-time inlined expansions.
Neither LD_PRELOAD nor patching glibc is going to catch the compile-time inlined
expansions. The compiler is going to need to add some check code to catch the problems
with the inlined expansions or people are going to need to compile code with
"-fno-builtin".
6. SIGSEGVs for violations from just-in-time compiled code: uninit
local
variables in backtrace_symbols() not set by calls to dl_iterate_phdr.
-Will
4793
days inactive
4804
days old
6 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jakub Jelinek -
John Reiser -
William Cohen