[PATCH] [FAT] fix VFAT compat ioctls on 64-bit systems

[Bart Oldeman]

Hello,

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 patch below fixes this. It's a minimal patch -- I haven't converted 
spaces to tabs or added extra put_user checks.

The problems:
* d_ino/d_off are undefined for de[0]. Random values from the kernel stack
  are copied 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>

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 13:59:45.000000000 -0400
@@ -749,14 +749,23 @@ 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 (__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 +796,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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/