[PATCH] Security,FAT: fix VFAT compat ioctls on 64-bit systems (2ndtry)

From: Bart Oldeman
Date: Sun Apr 29 2007 - 15:55:53 EST

I discovered another mistake with the patch sent before. Because there are
substantial 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 <bartoldeman@xxxxxxxxxxxxxxxxxxxxx>


#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);
return 0;


--- 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
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/