Bug 11333 - size of struct dirent does not agree with kernel when using LFS on 32bit
Summary: size of struct dirent does not agree with kernel when using LFS on 32bit
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.11
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-27 05:55 UTC by Kees Cook
Modified: 2014-06-30 18:44 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Makefile (533 bytes, application/octet-stream)
2010-02-27 06:16 UTC, Kees Cook
Details
test.c (374 bytes, text/x-csrc)
2010-02-27 06:16 UTC, Kees Cook
Details
test.c (489 bytes, text/x-csrc)
2010-02-27 06:38 UTC, Kees Cook
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kees Cook 2010-02-27 05:55:49 UTC
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;
}
Comment 1 Kees Cook 2010-02-27 06:16:17 UTC
Created attachment 4636 [details]
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
Comment 2 Kees Cook 2010-02-27 06:16:32 UTC
Created attachment 4637 [details]
test.c
Comment 3 Kees Cook 2010-02-27 06:38:30 UTC
Created attachment 4638 [details]
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.
Comment 4 Kees Cook 2010-02-27 07:16:39 UTC
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?
Comment 5 Ulrich Drepper 2010-04-04 06:54:36 UTC
You cannot change the data structure definition, that's an ABI change.

I've added code handling the memcpy.
Comment 7 Sunil 2010-05-26 21:56:00 UTC
Does this apply to earlier glibc versions as well?
Comment 8 Kees Cook 2010-05-26 23:07:14 UTC
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.