Re: [PATCH 17/25] vfat: move ioctl32 code to fs/fat/dir.c

[Christoph Hellwig]

> +#ifdef CONFIG_COMPAT
> +/* vfat */
> +#define	VFAT_IOCTL_READDIR_BOTH32	_IOR('r', 1, struct compat_dirent[2])
> +#define	VFAT_IOCTL_READDIR_SHORT32	_IOR('r', 2, struct compat_dirent[2])

these should be moved close to the original VFAT ioctl defintions. And
there's no need to put them under ifdef.

> +static long
> +put_dirent32(struct dirent *d, struct compat_dirent __user * d32)
> +{
> +	if (!access_ok(VERIFY_WRITE, d32, sizeof(struct compat_dirent)))
> +		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);

missing error checks.

> +static long fat_dir_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct compat_dirent __user *p = compat_ptr(arg);
> +	int ret;
> +	mm_segment_t oldfs = get_fs();
> +	struct dirent d[2];

please use proper compat_alloc_user_space here.

> +	switch (cmd) {
> +	case VFAT_IOCTL_READDIR_BOTH32:
> +		cmd = VFAT_IOCTL_READDIR_BOTH;
> +		break;
> +	case VFAT_IOCTL_READDIR_SHORT32:
> +		cmd = VFAT_IOCTL_READDIR_SHORT;
> +		break;
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +
> +	set_fs(KERNEL_DS);
> +	ret = fat_dir_ioctl(file->f_dentry->d_inode, file, cmd, (unsigned long) &d);
> +	set_fs(oldfs);

In fact there's even a much better way to implement this, let the
ioctls call __fat_readdir directly with a filldir callback that directly
works in compat_dirent structures.

-
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/