I discovered another mistake with the patch sent before. Because there aresubstantial leaks from the kernel stack into user space I am Cc-ing to security.
If you compile and run the below test case in an msdos or vfat directory on an x86-64 system with -m32 you'll get garbage in the kernel_dirent struct followed by a SIGSEGV. The SIGSEGV is caused by user stack corruption.
The patch below fixes this. It's a minimal patch -- I haven't converted spaces to tabs or added extra put_user checks.
The problems: * when the ioctl is called at the end of the directory (signalled via d[0].d_reclen==0), the d[1].d_reclen field is not initialized, but it is used as the length for copying the long filename into user space. This caused, in my case, the leakage of approximately 3000 bytes of kernel stack data into user space. * d_ino/d_off are undefined for de[0]. Random values from the kernel stack are copied from here into user space. * d_name, for both de[0] and de[1], is not zero terminated. * if the long filename in de[1] is empty, d_ino/d_off are also undefined for de[1]. Signed-off-by: Bart Oldeman <[email protected]> testcase: #include <sys/types.h> #include <sys/ioctl.h> #include <dirent.h> #include <stdio.h> #include <unistd.h> #include <fcntl.h> struct kernel_dirent { long d_ino; long d_off; unsigned short d_reclen; char d_name[256]; /* We must not include limits.h! */ }; #define VFAT_IOCTL_READDIR_BOTH _IOR('r', 1, struct kernel_dirent [2]) #define VFAT_IOCTL_READDIR_SHORT _IOR('r', 2, struct kernel_dirent [2]) int main(void) { int fd = open(".", O_RDONLY); struct kernel_dirent de[2]; while (1) { int i = ioctl(fd, VFAT_IOCTL_READDIR_BOTH, (long)de); if (i == -1) break; if (de[0].d_reclen == 0) break; printf("SFN: reclen=%2d off=%d ino=%d, %-12s", de[0].d_reclen, de[0].d_off, de[0].d_ino, de[0].d_name); if (de[1].d_reclen) printf("\tLFN: reclen=%2d off=%d ino=%d, %s", de[1].d_reclen, de[1].d_off, de[1].d_ino, de[1].d_name); printf("\n"); } return 0; } Patch: --- linux-2.6/fs/fat/dir.c.orig 2007-04-29 12:41:04.000000000 -0400 +++ linux-2.6/fs/fat/dir.c 2007-04-29 15:26:42.000000000 -0400 @@ -749,14 +749,25 @@ static int fat_dir_ioctl(struct inode * static long fat_compat_put_dirent32(struct dirent *d, struct compat_dirent __user *d32) { - if (!access_ok(VERIFY_WRITE, d32, sizeof(struct compat_dirent))) + if (!access_ok(VERIFY_WRITE, d32, sizeof(struct compat_dirent[2]))) return -EFAULT; - __put_user(d->d_ino, &d32->d_ino); - __put_user(d->d_off, &d32->d_off); __put_user(d->d_reclen, &d32->d_reclen); + __put_user(0, d32->d_name + d->d_reclen); + if (d->d_reclen == 0) + return 0; if (__copy_to_user(d32->d_name, d->d_name, d->d_reclen)) return -EFAULT; + d++; + d32++; + __put_user(d->d_reclen, &d32->d_reclen); + __put_user(0, d32->d_name + d->d_reclen); + if (d->d_reclen) { + __put_user(d->d_ino, &d32->d_ino); + __put_user(d->d_off, &d32->d_off); + if (__copy_to_user(d32->d_name, d->d_name, d->d_reclen)) + return -EFAULT; + } return 0; } @@ -787,8 +798,7 @@ static long fat_compat_dir_ioctl(struct unlock_kernel(); set_fs(oldfs); if (ret >= 0) { - ret |= fat_compat_put_dirent32(&d[0], p); - ret |= fat_compat_put_dirent32(&d[1], p + 1); + ret |= fat_compat_put_dirent32(d, p); } return ret; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
- Prev by Date: Re: [PATCH] [FAT] fix VFAT compat ioctls on 64-bit systems
- Next by Date: Re: Linux 2.6.21
- Previous by thread: 2.6.21-rc7-mm2 suspend bug. [kernel/kthread.c]
- Next by thread: Greylisting
- Index(es):
 |