Launchpad has imported 9 comments from the remote bug at http://sourceware.org/bugzilla/show_bug.cgi?id=11333.
If you reply to an imported comment from within Launchpad, your comment will be sent to the remote bug automatically. Read more about Launchpad's inter-bugtracker facilities at https://help.launchpad.net/InterBugTracking. ------------------------------------------------------------------------ On 2010-02-27T05:55:49+00:00 Kees Cook wrote: Forwarded from https://launchpad.net/bugs/392501 It seems that the actual size of "struct dirent" with LFS enabled is 280 bytes, but when defined for 32bit applications, the defined struct ends up at 276, and something (the kernel?) is still writing the remaining 4 bytes. Built on 64bit: cc -Wall -Werror -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -o test-native test.c cc -Wall -Werror -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -m32 -o test-m32 test.c mkdir -p bug-dir touch bug-dir/111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111 ./test-native bug-dir sizeof(struct dirent): 280 ./test-m32 bug-dir sizeof(struct dirent): 276 *** stack smashing detected ***: ./test-m32 terminated Built on 32bit: cc -Wall -Werror -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -o test-native test.c cc -Wall -Werror -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -m32 -o test-m32 test.c mkdir -p bug-dir touch bug-dir/111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111 ./test-native bug-dir sizeof(struct dirent): 276 *** stack smashing detected ***: ./test-native terminated /// test.c #include <stdio.h> #include <stdlib.h> #include <dirent.h> #include <inttypes.h> void func(const char*path) { struct dirent entry; struct dirent *result = NULL; int ret; DIR *dir = opendir(path); if(!dir) abort(); printf("sizeof(struct dirent): %" PRIuFAST32 "\n", sizeof(entry)); while (!(ret = readdir_r(dir, &entry, &result)) && result) {} } int main(int argc, const char** argv) { if(argc < 2) abort(); func(argv[1]); return 0; } Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/4 ------------------------------------------------------------------------ On 2010-02-27T06:16:17+00:00 Kees Cook wrote: Created attachment 4636 Makefile Line-wrapping did nasty things to the 255-character filename in the original bug description. Here is a Makefile and test.c that demonstrates the issue. What's really odd is that the 4 byte difference appears to be strictly padding? All the offsets and sizes are the same between 64bit and 32bit: ./test-native bug-dir sizeof(struct dirent): 280 sizeof(dirent.d_ino@0): 8 sizeof(dirent.d_off@8): 8 sizeof(dirent.d_reclen@16): 2 sizeof(dirent.d_type@18): 1 sizeof(dirent.d_name@19): 256 ./test-m32 bug-dir sizeof(struct dirent): 276 sizeof(dirent.d_ino@0): 8 sizeof(dirent.d_off@8): 8 sizeof(dirent.d_reclen@16): 2 sizeof(dirent.d_type@18): 1 sizeof(dirent.d_name@19): 256 *** stack smashing detected ***: ./test-m32 terminated Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/5 ------------------------------------------------------------------------ On 2010-02-27T06:16:32+00:00 Kees Cook wrote: Created attachment 4637 test.c Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/6 ------------------------------------------------------------------------ On 2010-02-27T06:38:30+00:00 Kees Cook wrote: Created attachment 4638 test.c This reports the reclen coming from the dirp->data. sysdeps/unix/readdir_r.c: bytes = __GETDENTS (dirp->fd, dirp->data, maxread); ... dp = (DIRENT_TYPE *) &dirp->data[dirp->offset]; ... reclen = dp->d_reclen; ... *result = memcpy (entry, dp, reclen); It seems that the memcpy is what overflows. I wonder if adding an "assert(sizeof(*entry) >= reclen)" should be added in here for fun, too. Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/7 ------------------------------------------------------------------------ On 2010-02-27T07:16:39+00:00 Kees Cook wrote: Looks like the kernel unconditionally aligns/pads to 8 bytes in the 64bit interface. fs/readdir.c, filldir64() says: int reclen = ALIGN(NAME_OFFSET(dirent) + namlen + 1, sizeof(u64)); which means it looks like alignment needs to be forced in glibc too. I don't think __attribute__ ((aligned (sizeof (__off64_t)))) is acceptable for bits/dirent.h as that's a gcc extension. Thoughts? Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/8 ------------------------------------------------------------------------ On 2010-04-04T06:54:36+00:00 Drepper-fsp wrote: You cannot change the data structure definition, that's an ABI change. I've added code handling the memcpy. Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/10 ------------------------------------------------------------------------ On 2010-04-04T17:55:21+00:00 Kees Cook wrote: Thanks! http://repo.or.cz/w/glibc.git/commitdiff/1a81139728494810f65aaa0d0c538ff8c2783dd5 Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/11 ------------------------------------------------------------------------ On 2010-05-26T21:56:00+00:00 devsk wrote: Does this apply to earlier glibc versions as well? Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/18 ------------------------------------------------------------------------ On 2010-05-26T23:07:14+00:00 Kees Cook wrote: Yes, this bug seems to have always existed. I checked back through ancient Linux kernel history, and it's always padded the dirent up to get the 64bit alignment. Reply at: https://bugs.launchpad.net/glibc/+bug/392501/comments/19 ** Changed in: glibc Importance: Unknown => Medium -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/392501 Title: readdir_r smashes stack on long dir entry -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
