linux-kernel - Re: [PATCH 5/7] staging:lustre: simplify libcfs_psdev

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160203224035.GA10529@kroah.com>
Date:	Wed, 3 Feb 2016 14:40:35 -0800
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	James Simmons <jsimmons@...radead.org>
Cc:	devel@...verdev.osuosl.org, Oleg Drokin <oleg.drokin@...el.com>,
	Andreas Dilger <andreas.dilger@...el.com>,
	Parinay Kondekar <parinay.kondekar@...gate.com>,
	James Simmons <uja.ornl@...oo.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	lustre-devel@...ts.lustre.org
Subject: Re: [PATCH 5/7] staging:lustre: simplify libcfs_psdev_[open|release]

On Tue, Jan 05, 2016 at 02:35:17PM -0500, James Simmons wrote:
> From: Parinay Kondekar <parinay.kondekar@...gate.com>
> 
> With struct libcfs_device_userstate gone we can move
> the remaining code of libcfs_psdev_ops.p_[open|close]
> into the libcfs_psdev_[open|release] functions directly.
> 
> Signed-off-by: Parinay Kondekar <parinay.kondekar@...gate.com>
> Signed-off-by: James Simmons <uja.ornl@...oo.com>
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5844
> Reviewed-on: http://review.whamcloud.com/17492
> Reviewed-by: Andreas Dilger <andreas.dilger@...el.com>
> Reviewed-by: Dmitry Eremin <dmitry.eremin@...el.com>
> Reviewed-by: John L. Hammond <john.hammond@...el.com>
> Reviewed-by: Oleg Drokin <oleg.drokin@...el.com>
> ---
>  .../staging/lustre/include/linux/libcfs/libcfs.h   |    2 --
>  .../lustre/lustre/libcfs/linux/linux-module.c      |   20 ++++++--------------
>  drivers/staging/lustre/lustre/libcfs/module.c      |   16 ----------------
>  3 files changed, 6 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> index 0d8a91e..06bb676 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> @@ -73,8 +73,6 @@ struct cfs_psdev_file {
>  };
>  
>  struct cfs_psdev_ops {
> -	int (*p_open)(unsigned long, void *);
> -	int (*p_close)(unsigned long, void *);
>  	int (*p_read)(struct cfs_psdev_file *, char *, unsigned long);
>  	int (*p_write)(struct cfs_psdev_file *, char *, unsigned long);
>  	int (*p_ioctl)(struct cfs_psdev_file *, unsigned long, void *);
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> index 33f6036..64f0fbf 100644
> --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> @@ -98,30 +98,22 @@ int libcfs_ioctl_popdata(void *arg, void *data, int size)
>  static int
>  libcfs_psdev_open(struct inode *inode, struct file *file)
>  {
> -	int    rc = 0;
> -
>  	if (!inode)
>  		return -EINVAL;
> -	if (libcfs_psdev_ops.p_open != NULL)
> -		rc = libcfs_psdev_ops.p_open(0, NULL);
> -	else
> -		return -EPERM;
> -	return rc;
> +
> +	try_module_get(THIS_MODULE);

Note, code like this is racy and incorrect and never needed, please fix
this up properly (hint, set the module in the file operations.)

Again, if you ever see code with that line, it is incorrect.

thanks,

greg k-h