severity 488467 grave
retitle 488467 libtrash is dangerously broken on amd64
tag 488467 patch
thanks
On Sun, Jun 29, 2008 at 09:25:13AM +0200, Mike Hommey wrote:
> On Sun, Jun 29, 2008 at 09:10:00AM +0200, Mike Hommey wrote:
> > Package: libtrash
> > Version: 2.4-1
> > Severity: important
> >
> > Interestingly, it doesn't crash under gdb, but fortunately, it does
> > under valgrind.
> >
> > Here is the stacktrace:
> > ==17769== Jump to the invalid address stated on the next line
> > ==17769== at 0x0: ???
> > ==17769== by 0x40235DD: (within /usr/lib/libtrash/libtrash.so.2.4)
> > ==17769== by 0x4023C05: fopen (in /usr/lib/libtrash/libtrash.so.2.4)
> > ==17769== by 0x67944DA: internal_setpwent (compat-pwd.c:244)
> > ==17769== by 0x6794FB9: _nss_compat_getpwuid_r (compat-pwd.c:1114)
> > ==17769== by 0x54ED301: getpwuid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:226)
> > ==17769== by 0x54ECBCE: getpwuid (getXXbyYY.c:116)
> > ==17769== by 0x40B673: (within /bin/ls)
> > ==17769== by 0x4031A4: (within /bin/ls)
> > ==17769== by 0x403DE0: (within /bin/ls)
> > ==17769== by 0x4062C6: (within /bin/ls)
> > ==17769== by 0x40737F: (within /bin/ls)
> > ==17769== Address 0x0 is not stack'd, malloc'd or (recently) free'd
>
> Here is the culprit:
> real_fopen = dlvsym(RTLD_NEXT, "fopen", "GLIBC_2.1");
>
> There is no such version of the symbol:
> $ objdump -T /lib/libc.so.6 | grep ' fopen$'
> 0000000000064840 g DF .text 000000000000000a GLIBC_2.2.5 fopen
It's even worse than that. Anything using other f* functions (freopen,
fopen64, etc.) will get a wrong return value because of bad declarations
for the function pointers.
The attached patch should fix both issues. It doesn't fix more of the
brain damage in the code source itself (dlsym()ing at every call is
stupid, having only one function to do the whole thing is bad design,
and macros that are used only once don't help making the code readable)
Although this patch makes things worse, I strongly advise to audit the
code for other similar "jokes". (Or, even better, a complete rewrite)
Mike
--- libtrash-2.4.orig/src/open-funs.c 2008-06-29 13:32:30.757873567 +0200
+++ libtrash-2.4/src/open-funs.c 2008-06-29 13:45:22.529884562 +0200
@@ -389,18 +389,20 @@
/* the call to dlvsym() in the particular case of fopen() fixes a funny
interaction with GNU libc. When dlsym() was used instead of dlvsym(), GNU libc
would give us a pointer to an older version of fopen() and subsequently crash
if the calling code tried to run, e.g., getwc() on the returned FILE* pointer.
*/
#define GET_REAL_FUNCTION()
\
- int (*real_fopen) (const char *path, const char *mode) = NULL;
\
- int (*real_fopen64) (const char *path, const char *mode) = NULL;
\
- int (*real_freopen) (const char *path, const char *mode, FILE *stream) =
NULL; \
- int (*real_freopen64) (const char *path, const char *mode, FILE *stream) =
NULL; \
+ FILE *(*real_fopen) (const char *path, const char *mode) = NULL;
\
+ FILE *(*real_fopen64) (const char *path, const char *mode) = NULL;
\
+ FILE *(*real_freopen) (const char *path, const char *mode, FILE *stream) =
NULL; \
+ FILE *(*real_freopen64) (const char *path, const char *mode, FILE *stream)
= NULL; \
int (*real_open) (const char *path, int flags, ...) = NULL;
\
int (*real_open64) (const char *path, int flags, ...) = NULL;
\
\
dlerror();
\
\
- if (function == FOPEN)
\
+ if (function == FOPEN) {
\
real_fopen = dlvsym(RTLD_NEXT, "fopen", "GLIBC_2.1");
\
- else if (function == FOPEN64)
\
+ if (!real_fopen)
\
+ real_fopen = dlsym(RTLD_NEXT, "fopen");
\
+ } else if (function == FOPEN64)
\
real_fopen64 = dlsym(RTLD_NEXT, "fopen64");
\
else if (function == FREOPEN)
\
real_freopen = dlsym(RTLD_NEXT, "freopen");
\
