linux-kernel - Re: [PATCH] cifs: Push the BKL down into the fs ioctl code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <524f69650805221857p7ff6e539m18902362b1d30a72@mail.gmail.com>
Date:	Thu, 22 May 2008 20:57:48 -0500
From:	"Steve French" <smfrench@...il.com>
To:	"Alan Cox" <alan@...rguk.ukuu.org.uk>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cifs: Push the BKL down into the fs ioctl code

cifs already converted to unlocked ioctl last week.

On Thu, May 22, 2008 at 4:52 PM, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
> Switch to unlocked_ioctl. Wrap the ioctl handler internally with
> lock_kernel until someone can prove it isn't needed or fix it so it isn't.
>
> Signed-off-by: Alan Cox <alan@...hat.com>
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 427a7c6..b6436b8 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -657,7 +657,7 @@ const struct file_operations cifs_file_ops = {
>        .splice_read = generic_file_splice_read,
>        .llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_POSIX
> -       .ioctl  = cifs_ioctl,
> +       .unlocked_ioctl = cifs_ioctl,
>  #endif /* CONFIG_CIFS_POSIX */
>
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
> @@ -677,7 +677,7 @@ const struct file_operations cifs_file_direct_ops = {
>        .flush = cifs_flush,
>        .splice_read = generic_file_splice_read,
>  #ifdef CONFIG_CIFS_POSIX
> -       .ioctl  = cifs_ioctl,
> +       .unlocked_ioctl  = cifs_ioctl,
>  #endif /* CONFIG_CIFS_POSIX */
>        .llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
> @@ -697,7 +697,7 @@ const struct file_operations cifs_file_nobrl_ops = {
>        .splice_read = generic_file_splice_read,
>        .llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_POSIX
> -       .ioctl  = cifs_ioctl,
> +       .unlocked_ioctl = cifs_ioctl,
>  #endif /* CONFIG_CIFS_POSIX */
>
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
> @@ -716,7 +716,7 @@ const struct file_operations cifs_file_direct_nobrl_ops = {
>        .flush = cifs_flush,
>        .splice_read = generic_file_splice_read,
>  #ifdef CONFIG_CIFS_POSIX
> -       .ioctl  = cifs_ioctl,
> +       .unlocked_ioctl  = cifs_ioctl,
>  #endif /* CONFIG_CIFS_POSIX */
>        .llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
> @@ -731,7 +731,7 @@ const struct file_operations cifs_dir_ops = {
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>        .dir_notify = cifs_dir_notify,
>  #endif /* CONFIG_CIFS_EXPERIMENTAL */
> -       .ioctl  = cifs_ioctl,
> +       .unlocked_ioctl  = cifs_ioctl,
>  };
>
>  static void
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index cd1301a..1312ae7 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -95,8 +95,8 @@ extern int    cifs_setxattr(struct dentry *, const char *, const void *,
>                        size_t, int);
>  extern ssize_t cifs_getxattr(struct dentry *, const char *, void *, size_t);
>  extern ssize_t cifs_listxattr(struct dentry *, char *, size_t);
> -extern int cifs_ioctl(struct inode *inode, struct file *filep,
> -                      unsigned int command, unsigned long arg);
> +extern long cifs_ioctl(struct file *filep, unsigned int command,
> +                                                       unsigned long arg);
>
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  extern const struct export_operations cifs_export_ops;
> diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
> index 5c792df..396811f 100644
> --- a/fs/cifs/ioctl.c
> +++ b/fs/cifs/ioctl.c
> @@ -22,6 +22,7 @@
>  */
>
>  #include <linux/fs.h>
> +#include <linux/smp_lock.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
>  #include "cifsproto.h"
> @@ -30,9 +31,9 @@
>
>  #define CIFS_IOC_CHECKUMOUNT _IO(0xCF, 2)
>
> -int cifs_ioctl(struct inode *inode, struct file *filep,
> -               unsigned int command, unsigned long arg)
> +long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
>  {
> +       struct inode *inode = filep->f_path.dentry->d_inode;
>        int rc = -ENOTTY; /* strange error - but the precedent */
>        int xid;
>        struct cifs_sb_info *cifs_sb;
> @@ -45,6 +46,8 @@ int cifs_ioctl(struct inode *inode, struct file *filep,
>                (struct cifsFileInfo *)filep->private_data;
>  #endif /* CONFIG_CIFS_POSIX */
>
> +       lock_kernel();
> +
>        xid = GetXid();
>
>        cFYI(1, ("ioctl file %p  cmd %u  arg %lu", filep, command, arg));
> @@ -58,6 +61,7 @@ int cifs_ioctl(struct inode *inode, struct file *filep,
>        else {
>                rc = -EIO;
>                FreeXid(xid);
> +               unlock_kernel();
>                return -EIO;
>        }
>  #endif /* CONFIG_CIFS_POSIX */
> @@ -106,5 +110,6 @@ int cifs_ioctl(struct inode *inode, struct file *filep,
>        }
>
>        FreeXid(xid);
> +       unlock_kernel();
>        return rc;
>  }
>



-- 
Thanks,

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