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]
Date:	Thu, 18 Dec 2008 00:49:19 -0600
From:	Felix Blyakher <felixb@....com>
To:	Ankit Jain <me@...itjain.org>
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	Christoph Hellwig <hch@...radead.org>,
	linux-fsdevel@...r.kernel.org, mfasheh@...e.com,
	joel.becker@...cle.com, ocfs2-devel@....oracle.com,
	linux-kernel@...r.kernel.org, xfs-masters@....sgi.com,
	xfs@....sgi.com
Subject: Re: [PATCH][RFC] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls


On Dec 15, 2008, at 2:04 AM, Ankit Jain wrote:

> This patch adds ioctls to vfs for compatibility with legacy XFS
> pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
> effectively invokes sys_fallocate for the new ioctls.

I don't think we can use sys_fallocate for XFS_IOC_UNRESVSP*
commands, which suppose to release currently allocated file
blocks.
See more comments below.

Felix

> Note: These legacy ioctls are also implemented by OCFS2.
>
> There are some things that I'm not sure about:
> 1. Should the struct space_resv be exposed to user-space? If not,
> then what would be the right place to put it? And the ioctl
> definitions?
> 2. Should the corresponding ioctls be removed from ocfs2?
>
> Signed-off-by: Ankit Jain <me@...itjain.org>
> ---
>  fs/ioctl.c             |   37 +++++++++++++++++++++++++++
>  fs/open.c              |   51 ++++++++++++++++++-------------------
>  include/linux/falloc.h |   19 ++++++++++++++
>  include/linux/fs.h     |    2 +
>  4 files changed, 83 insertions(+), 26 deletions(-)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 43e8b2c..5e565c8 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -15,6 +15,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/writeback.h>
>  #include <linux/buffer_head.h>
> +#include <linux/falloc.h>
>
>  #include <asm/ioctls.h>
>
> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>
>  #endif  /*  CONFIG_BLOCK  */
>
> +/*
> + * This provides compatibility with legacy XFS pre-allocation ioctls
> + * which predate the fallocate syscall.
> + *
> + * Only the l_start, l_len and l_whence fields of the 'struct  
> space_resv'
> + * are used here, rest are ignored.
> + */
> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
> +{
> +	struct inode *inode = filp->f_path.dentry->d_inode;
> +	struct space_resv sr;
> +
> +	if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof 
> (sr)))
> +		return -EFAULT;
> +
> +	switch (sr.l_whence) {
> +	case SEEK_SET:
> +		break;
> +	case SEEK_CUR:
> +		sr.l_start += filp->f_pos;
> +		break;
> +	case SEEK_END:
> +		sr.l_start += i_size_read(inode);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start,  
> sr.l_len);
> +}
> +
>  static int file_ioctl(struct file *filp, unsigned int cmd,
>  		unsigned long arg)
>  {
> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp,  
> unsigned int cmd,
>  		return put_user(inode->i_sb->s_blocksize, p);
>  	case FIONREAD:
>  		return put_user(i_size_read(inode) - filp->f_pos, p);
> +	case F_IOC_RESVSP:
> +	case F_IOC_RESVSP64:
> +	case F_IOC_UNRESVSP:
> +	case F_IOC_UNRESVSP64:
> +		return ioctl_preallocate(filp, arg);

At this point the original command 'cmd' is dropped,
and ioctl_preallocate() assumes F_IOC_RESVSP*.
Indeed, in the following path

     ioctl_preallocate
         do_fallocate
             .fallocate/xfs_vn_fallocate
                 xfs_change_file_space(XFS_IOC_RESVSP)

UNRESVSP is never considered.

>  	}
>
>  	return vfs_ioctl(filp, cmd, arg);
> diff --git a/fs/open.c b/fs/open.c
> index 83cdb9d..0703bcb 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -360,62 +360,61 @@ asmlinkage long sys_ftruncate64(unsigned int  
> fd, loff_t length)
>  }
>  #endif
>
> -asmlinkage long sys_fallocate(int fd, int mode, loff_t offset,  
> loff_t len)
> +long do_fallocate(struct file *file, int mode, loff_t offset,  
> loff_t len)
>  {
> -	struct file *file;
>  	struct inode *inode;
> -	long ret = -EINVAL;
> +	long ret;
>
>  	if (offset < 0 || len <= 0)
> -		goto out;
> +		return -EINVAL;
>
>  	/* Return error if mode is not supported */
> -	ret = -EOPNOTSUPP;
>  	if (mode && !(mode & FALLOC_FL_KEEP_SIZE))
> -		goto out;
> +		return -EOPNOTSUPP;
>
> -	ret = -EBADF;
> -	file = fget(fd);
> -	if (!file)
> -		goto out;
> -	if (!(file->f_mode & FMODE_WRITE))
> -		goto out_fput;
> +	if (!file || !(file->f_mode & FMODE_WRITE))
> +		return -EBADF;
>  	/*
>  	 * Revalidate the write permissions, in case security policy has
>  	 * changed since the files were opened.
>  	 */
>  	ret = security_file_permission(file, MAY_WRITE);
>  	if (ret)
> -		goto out_fput;
> +		return ret;
>
>  	inode = file->f_path.dentry->d_inode;
> -
> -	ret = -ESPIPE;
>  	if (S_ISFIFO(inode->i_mode))
> -		goto out_fput;
> +		return -ESPIPE;
>
> -	ret = -ENODEV;
>  	/*
>  	 * Let individual file system decide if it supports preallocation
>  	 * for directories or not.
>  	 */
>  	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> -		goto out_fput;
> +		return -ENODEV;
>
> -	ret = -EFBIG;
>  	/* Check for wrap through zero too */
>  	if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len)  
> < 0))
> -		goto out_fput;
> +		return -EFBIG;
>
>  	if (inode->i_op && inode->i_op->fallocate)
> -		ret = inode->i_op->fallocate(inode, mode, offset, len);
> +		return inode->i_op->fallocate(inode, mode, offset, len);
>  	else
> -		ret = -EOPNOTSUPP;
> +		return -EOPNOTSUPP;
> +}
>
> -out_fput:
> -	fput(file);
> -out:
> -	return ret;
> +asmlinkage long sys_fallocate(int fd, int mode, loff_t offset,  
> loff_t len)
> +{
> +	struct file *file;
> +	int error = -EBADF;
> +
> +	file = fget(fd);
> +	if (file) {
> +		error = do_fallocate(file, mode, offset, len);
> +		fput(file);
> +	}
> +
> +	return error;
>  }
>
>  /*
> diff --git a/include/linux/falloc.h b/include/linux/falloc.h
> index 8e912ab..4f2a727 100644
> --- a/include/linux/falloc.h
> +++ b/include/linux/falloc.h
> @@ -3,4 +3,23 @@
>
>  #define FALLOC_FL_KEEP_SIZE	0x01 /* default is extend size */
>
> +/*
> + * Space reservation / allocation ioctls and argument structure
> + * are designed to be compatible with the legacy XFS ioctls.
> + */
> +struct space_resv {
> +	__s16		l_type;
> +	__s16		l_whence;
> +	__s64		l_start;
> +	__s64		l_len;		/* len == 0 means until end of file */
> +	__s32		l_sysid;
> +	__u32		l_pid;
> +	__s32		l_pad[4];	/* reserve area			    */
> +};
> +
> +#define F_IOC_RESVSP		_IOW('X', 40, struct space_resv)
> +#define F_IOC_UNRESVSP	_IOW('X', 41, struct space_resv)
> +#define F_IOC_RESVSP64	_IOW('X', 42, struct space_resv)
> +#define F_IOC_UNRESVSP64	_IOW('X', 43, struct space_resv)
> +
>  #endif /* _FALLOC_H_ */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 4a853ef..b1d8f12 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1671,6 +1671,8 @@ static inline int break_lease(struct inode  
> *inode, unsigned int mode)
>
>  extern int do_truncate(struct dentry *, loff_t start, unsigned int  
> time_attrs,
>  		       struct file *filp);
> +extern long do_fallocate(struct file *file, int mode, loff_t offset,
> +			loff_t len);
>  extern long do_sys_open(int dfd, const char __user *filename, int  
> flags,
>  			int mode);
>  extern struct file *filp_open(const char *, int, int);
> --
> To unsubscribe from this list: send the line "unsubscribe linux- 
> fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Powered by Openwall GNU/*/Linux Powered by OpenVZ