| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 06 May 2010 01:04:37 +0900
From: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To: John Kacur <jkacur@...hat.com>
Cc: lkml <linux-kernel@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...e.hu>,
Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 3/6] fat: BKL ioctl pushdown
John Kacur <jkacur@...hat.com> writes:
> Convert fat_generic_ioctl and fat_dir_ioctl to unlocked_ioctls
> and push down the bkl into those functions.
I guess this is the part of batch ioctl conversion stuff though, those
ioctl of FAT don't need BKL at all. Because all of those should already
be protected by inode->i_mutex.
Removing BKL and then cleanup after this patch would be almost same with
reverting this patch. So, could you just convert to unlocked_ioctl
instead?
Thanks.
> Signed-off-by: John Kacur <jkacur@...hat.com>
> ---
> fs/fat/dir.c | 36 ++++++++++++++++++++++++------------
> fs/fat/fat.h | 3 +--
> fs/fat/file.c | 22 ++++++++++++++++------
> 3 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 530b4ca..1b73e60 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -19,6 +19,7 @@
> #include <linux/buffer_head.h>
> #include <linux/compat.h>
> #include <asm/uaccess.h>
> +#include <linux/smp_lock.h>
> #include "fat.h"
>
> /*
> @@ -737,10 +738,11 @@ efault: \
>
> FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_dirent)
>
> -static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
> +static long fat_ioctl_readdir(struct file *filp,
> void __user *dirent, filldir_t filldir,
> int short_only, int both)
> {
> + struct inode *inode = filp->f_dentry->d_inode;
> struct fat_ioctl_filldir_callback buf;
> int ret;
>
> @@ -758,9 +760,10 @@ static int fat_ioctl_readdir(struct inode *inode, struct file *filp,
> return ret;
> }
>
> -static int fat_dir_ioctl(struct inode *inode, struct file *filp,
> - unsigned int cmd, unsigned long arg)
> +static long fat_dir_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> {
> + long error;
> struct __fat_dirent __user *d1 = (struct __fat_dirent __user *)arg;
> int short_only, both;
>
> @@ -774,21 +777,31 @@ static int fat_dir_ioctl(struct inode *inode, struct file *filp,
> both = 1;
> break;
> default:
> - return fat_generic_ioctl(inode, filp, cmd, arg);
> + lock_kernel();
> + error = fat_generic_ioctl(filp, cmd, arg);
> + goto out;
> }
>
> - if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_dirent[2])))
> - return -EFAULT;
> + lock_kernel();
> + if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_dirent[2]))) {
> + error = -EFAULT;
> + goto out;
> + }
> /*
> * 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;
> + if (put_user(0, &d1->d_reclen)) {
> + error = -EFAULT;
> + goto out;
> + }
>
> - return fat_ioctl_readdir(inode, filp, d1, fat_ioctl_filldir,
> + error = fat_ioctl_readdir(filp, d1, fat_ioctl_filldir,
> short_only, both);
> +out:
> + unlock_kernel();
> + return error;
> }
>
> #ifdef CONFIG_COMPAT
> @@ -800,7 +813,6 @@ FAT_IOCTL_FILLDIR_FUNC(fat_compat_ioctl_filldir, compat_dirent)
> static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd,
> unsigned long arg)
> {
> - struct inode *inode = filp->f_path.dentry->d_inode;
> struct compat_dirent __user *d1 = compat_ptr(arg);
> int short_only, both;
>
> @@ -827,7 +839,7 @@ static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd,
> if (put_user(0, &d1->d_reclen))
> return -EFAULT;
>
> - return fat_ioctl_readdir(inode, filp, d1, fat_compat_ioctl_filldir,
> + return fat_ioctl_readdir(filp, d1, fat_compat_ioctl_filldir,
> short_only, both);
> }
> #endif /* CONFIG_COMPAT */
> @@ -836,7 +848,7 @@ const struct file_operations fat_dir_operations = {
> .llseek = generic_file_llseek,
> .read = generic_read_dir,
> .readdir = fat_readdir,
> - .ioctl = fat_dir_ioctl,
> + .unlocked_ioctl = fat_dir_ioctl,
> #ifdef CONFIG_COMPAT
> .compat_ioctl = fat_compat_dir_ioctl,
> #endif
> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index e6efdfa..23f9b2a 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -298,8 +298,7 @@ extern int fat_free_clusters(struct inode *inode, int cluster);
> extern int fat_count_free_clusters(struct super_block *sb);
>
> /* fat/file.c */
> -extern int fat_generic_ioctl(struct inode *inode, struct file *filp,
> - unsigned int cmd, unsigned long arg);
> +extern long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> extern const struct file_operations fat_file_operations;
> extern const struct inode_operations fat_file_inode_operations;
> extern int fat_setattr(struct dentry * dentry, struct iattr * attr);
> diff --git a/fs/fat/file.c b/fs/fat/file.c
> index e8c159d..4f3044f 100644
> --- a/fs/fat/file.c
> +++ b/fs/fat/file.c
> @@ -16,6 +16,7 @@
> #include <linux/blkdev.h>
> #include <linux/fsnotify.h>
> #include <linux/security.h>
> +#include <linux/smp_lock.h>
> #include "fat.h"
>
> static int fat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr)
> @@ -114,19 +115,28 @@ out:
> return err;
> }
>
> -int fat_generic_ioctl(struct inode *inode, struct file *filp,
> - unsigned int cmd, unsigned long arg)
> +long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> {
> + long error;
> + struct inode *inode = filp->f_dentry->d_inode;
> u32 __user *user_attr = (u32 __user *)arg;
>
> + lock_kernel();
> +
> switch (cmd) {
> case FAT_IOCTL_GET_ATTRIBUTES:
> - return fat_ioctl_get_attributes(inode, user_attr);
> + error = fat_ioctl_get_attributes(inode, user_attr);
> + break;
> case FAT_IOCTL_SET_ATTRIBUTES:
> - return fat_ioctl_set_attributes(filp, user_attr);
> + error = fat_ioctl_set_attributes(filp, user_attr);
> + break;
> default:
> - return -ENOTTY; /* Inappropriate ioctl for device */
> + error = -ENOTTY; /* Inappropriate ioctl for device */
> + break;
> }
> +
> + unlock_kernel();
> + return error;
> }
>
> static int fat_file_release(struct inode *inode, struct file *filp)
> @@ -159,7 +169,7 @@ const struct file_operations fat_file_operations = {
> .aio_write = generic_file_aio_write,
> .mmap = generic_file_mmap,
> .release = fat_file_release,
> - .ioctl = fat_generic_ioctl,
> + .unlocked_ioctl = fat_generic_ioctl,
> .fsync = fat_file_fsync,
> .splice_read = generic_file_splice_read,
> };
--
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists