-stable review patch. If anyone has any objections, please let us know.
---------------------
From: OGAWA Hirofumi <[email protected]>
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 fixes this.
Reported and initial fix by Bart Oldeman
#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;
}
Signed-off-by: Bart Oldeman <[email protected]>
Signed-off-by: OGAWA Hirofumi <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
fs/fat/dir.c | 199 +++++++++++++++++++++++++++++------------------------------
1 file changed, 100 insertions(+), 99 deletions(-)
--- linux-2.6.21.1.orig/fs/fat/dir.c
+++ linux-2.6.21.1/fs/fat/dir.c
@@ -422,7 +422,7 @@ EODir:
EXPORT_SYMBOL_GPL(fat_search_long);
struct fat_ioctl_filldir_callback {
- struct dirent __user *dirent;
+ void __user *dirent;
int result;
/* for dir ioctl */
const char *longname;
@@ -647,62 +647,85 @@ static int fat_readdir(struct file *filp
return __fat_readdir(inode, filp, dirent, filldir, 0, 0);
}
-static int fat_ioctl_filldir(void *__buf, const char *name, int name_len,
- loff_t offset, u64 ino, unsigned int d_type)
+#define FAT_IOCTL_FILLDIR_FUNC(func, dirent_type) \
+static int func(void *__buf, const char *name, int name_len, \
+ loff_t offset, u64 ino, unsigned int d_type) \
+{ \
+ struct fat_ioctl_filldir_callback *buf = __buf; \
+ struct dirent_type __user *d1 = buf->dirent; \
+ struct dirent_type __user *d2 = d1 + 1; \
+ \
+ if (buf->result) \
+ return -EINVAL; \
+ buf->result++; \
+ \
+ if (name != NULL) { \
+ /* dirent has only short name */ \
+ if (name_len >= sizeof(d1->d_name)) \
+ name_len = sizeof(d1->d_name) - 1; \
+ \
+ if (put_user(0, d2->d_name) || \
+ put_user(0, &d2->d_reclen) || \
+ copy_to_user(d1->d_name, name, name_len) || \
+ put_user(0, d1->d_name + name_len) || \
+ put_user(name_len, &d1->d_reclen)) \
+ goto efault; \
+ } else { \
+ /* dirent has short and long name */ \
+ const char *longname = buf->longname; \
+ int long_len = buf->long_len; \
+ const char *shortname = buf->shortname; \
+ int short_len = buf->short_len; \
+ \
+ if (long_len >= sizeof(d1->d_name)) \
+ long_len = sizeof(d1->d_name) - 1; \
+ if (short_len >= sizeof(d1->d_name)) \
+ short_len = sizeof(d1->d_name) - 1; \
+ \
+ if (copy_to_user(d2->d_name, longname, long_len) || \
+ put_user(0, d2->d_name + long_len) || \
+ put_user(long_len, &d2->d_reclen) || \
+ put_user(ino, &d2->d_ino) || \
+ put_user(offset, &d2->d_off) || \
+ copy_to_user(d1->d_name, shortname, short_len) || \
+ put_user(0, d1->d_name + short_len) || \
+ put_user(short_len, &d1->d_reclen)) \
+ goto efault; \
+ } \
+ return 0; \
+efault: \
+ buf->result = -EFAULT; \
+ return -EFAULT; \
+}
+
+FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, dirent)
+
+static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
+ void __user *dirent, filldir_t filldir,
+ int short_only, int both)
{
- struct fat_ioctl_filldir_callback *buf = __buf;
- struct dirent __user *d1 = buf->dirent;
- struct dirent __user *d2 = d1 + 1;
-
- if (buf->result)
- return -EINVAL;
- buf->result++;
-
- if (name != NULL) {
- /* dirent has only short name */
- if (name_len >= sizeof(d1->d_name))
- name_len = sizeof(d1->d_name) - 1;
-
- if (put_user(0, d2->d_name) ||
- put_user(0, &d2->d_reclen) ||
- copy_to_user(d1->d_name, name, name_len) ||
- put_user(0, d1->d_name + name_len) ||
- put_user(name_len, &d1->d_reclen))
- goto efault;
- } else {
- /* dirent has short and long name */
- const char *longname = buf->longname;
- int long_len = buf->long_len;
- const char *shortname = buf->shortname;
- int short_len = buf->short_len;
-
- if (long_len >= sizeof(d1->d_name))
- long_len = sizeof(d1->d_name) - 1;
- if (short_len >= sizeof(d1->d_name))
- short_len = sizeof(d1->d_name) - 1;
-
- if (copy_to_user(d2->d_name, longname, long_len) ||
- put_user(0, d2->d_name + long_len) ||
- put_user(long_len, &d2->d_reclen) ||
- put_user(ino, &d2->d_ino) ||
- put_user(offset, &d2->d_off) ||
- copy_to_user(d1->d_name, shortname, short_len) ||
- put_user(0, d1->d_name + short_len) ||
- put_user(short_len, &d1->d_reclen))
- goto efault;
+ struct fat_ioctl_filldir_callback buf;
+ int ret;
+
+ buf.dirent = dirent;
+ buf.result = 0;
+ mutex_lock(&inode->i_mutex);
+ ret = -ENOENT;
+ if (!IS_DEADDIR(inode)) {
+ ret = __fat_readdir(inode, filp, &buf, filldir,
+ short_only, both);
}
- return 0;
-efault:
- buf->result = -EFAULT;
- return -EFAULT;
+ mutex_unlock(&inode->i_mutex);
+ if (ret >= 0)
+ ret = buf.result;
+ return ret;
}
-static int fat_dir_ioctl(struct inode * inode, struct file * filp,
- unsigned int cmd, unsigned long arg)
+static int fat_dir_ioctl(struct inode *inode, struct file *filp,
+ unsigned int cmd, unsigned long arg)
{
- struct fat_ioctl_filldir_callback buf;
- struct dirent __user *d1;
- int ret, short_only, both;
+ struct dirent __user *d1 = (struct dirent __user *)arg;
+ int short_only, both;
switch (cmd) {
case VFAT_IOCTL_READDIR_SHORT:
@@ -717,7 +740,6 @@ static int fat_dir_ioctl(struct inode *
return fat_generic_ioctl(inode, filp, cmd, arg);
}
- d1 = (struct dirent __user *)arg;
if (!access_ok(VERIFY_WRITE, d1, sizeof(struct dirent[2])))
return -EFAULT;
/*
@@ -728,69 +750,48 @@ static int fat_dir_ioctl(struct inode *
if (put_user(0, &d1->d_reclen))
return -EFAULT;
- buf.dirent = d1;
- buf.result = 0;
- mutex_lock(&inode->i_mutex);
- ret = -ENOENT;
- if (!IS_DEADDIR(inode)) {
- ret = __fat_readdir(inode, filp, &buf, fat_ioctl_filldir,
- short_only, both);
- }
- mutex_unlock(&inode->i_mutex);
- if (ret >= 0)
- ret = buf.result;
- return ret;
+ return fat_ioctl_readdir(inode, filp, d1, fat_ioctl_filldir,
+ short_only, both);
}
#ifdef CONFIG_COMPAT
#define VFAT_IOCTL_READDIR_BOTH32 _IOR('r', 1, struct compat_dirent[2])
#define VFAT_IOCTL_READDIR_SHORT32 _IOR('r', 2, struct compat_dirent[2])
-static long fat_compat_put_dirent32(struct dirent *d,
- struct compat_dirent __user *d32)
-{
- if (!access_ok(VERIFY_WRITE, d32, sizeof(struct compat_dirent)))
- return -EFAULT;
+FAT_IOCTL_FILLDIR_FUNC(fat_compat_ioctl_filldir, compat_dirent)
- __put_user(d->d_ino, &d32->d_ino);
- __put_user(d->d_off, &d32->d_off);
- __put_user(d->d_reclen, &d32->d_reclen);
- if (__copy_to_user(d32->d_name, d->d_name, d->d_reclen))
- return -EFAULT;
-
- return 0;
-}
-
-static long fat_compat_dir_ioctl(struct file *file, unsigned cmd,
+static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd,
unsigned long arg)
{
- struct compat_dirent __user *p = compat_ptr(arg);
- int ret;
- mm_segment_t oldfs = get_fs();
- struct dirent d[2];
+ struct inode *inode = filp->f_path.dentry->d_inode;
+ struct compat_dirent __user *d1 = compat_ptr(arg);
+ int short_only, both;
switch (cmd) {
- case VFAT_IOCTL_READDIR_BOTH32:
- cmd = VFAT_IOCTL_READDIR_BOTH;
- break;
case VFAT_IOCTL_READDIR_SHORT32:
- cmd = VFAT_IOCTL_READDIR_SHORT;
+ short_only = 1;
+ both = 0;
+ break;
+ case VFAT_IOCTL_READDIR_BOTH32:
+ short_only = 0;
+ both = 1;
break;
default:
return -ENOIOCTLCMD;
}
- set_fs(KERNEL_DS);
- lock_kernel();
- ret = fat_dir_ioctl(file->f_path.dentry->d_inode, file,
- cmd, (unsigned long) &d);
- 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);
- }
- return ret;
+ if (!access_ok(VERIFY_WRITE, d1, sizeof(struct compat_dirent[2])))
+ return -EFAULT;
+ /*
+ * Yes, we don't need this put_user() absolutely. However old
+ * code didn't return the right value. So, app use this value,
+ * in order to check whether it is EOF.
+ */
+ if (put_user(0, &d1->d_reclen))
+ return -EFAULT;
+
+ return fat_ioctl_readdir(inode, filp, d1, fat_compat_ioctl_filldir,
+ short_only, both);
}
#endif /* CONFIG_COMPAT */
--