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, 15 Jun 2017 09:57:35 +0200
From:   Jan Kara <jack@...e.cz>
To:     Tahsin Erdogan <tahsin@...gle.com>
Cc:     Jan Kara <jack@...e.com>, Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Dave Kleikamp <shaggy@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Mark Fasheh <mfasheh@...sity.com>,
        Joel Becker <jlbec@...lplan.org>, Jens Axboe <axboe@...com>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Mike Christie <mchristi@...hat.com>,
        Fabian Frederick <fabf@...net.be>, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, jfs-discussion@...ts.sourceforge.net,
        linux-fsdevel@...r.kernel.org, ocfs2-devel@....oracle.com,
        reiserfs-devel@...r.kernel.org
Subject: Re: [PATCH 28/28] quota: add extra inode count to dquot transfer
 functions

On Wed 31-05-17 01:15:17, Tahsin Erdogan wrote:
> Ext4 ea_inode feature allows storing xattr values in external inodes to
> be able to store values that are bigger than a block in size. Ext4 also
> has deduplication support for these type of inodes. With deduplication,
> the actual storage waste is eliminated but the users of such inodes are
> still charged full quota for the inodes as if there was no sharing
> happening in the background.
> 
> This design requires ext4 to manually charge the users because the
> inodes are shared.
> 
> An implication of this is that, if someone calls chown on a file that
> has such references we need to transfer the quota for the file and xattr
> inodes. Current dquot_transfer() function implicitly transfers one inode
> charge. In our case, we would like to specify additional inodes to be
> transferred.

Hum, rather handle this similarly to how we handle delalloc reserved space.
Add a callback to dq_ops to get "inode usage" of an inode and then use it
in dquot_transfer(), dquot_free_inode(), dquot_alloc_inode().

								Honza

 
> Signed-off-by: Tahsin Erdogan <tahsin@...gle.com>
> ---
>  fs/ext2/inode.c          |  2 +-
>  fs/ext4/inode.c          |  8 ++++++-
>  fs/ext4/ioctl.c          | 13 +++++++++++-
>  fs/ext4/xattr.c          | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/ext4/xattr.h          |  2 ++
>  fs/jfs/file.c            |  2 +-
>  fs/ocfs2/file.c          |  2 +-
>  fs/quota/dquot.c         | 16 +++++++-------
>  fs/reiserfs/inode.c      |  2 +-
>  include/linux/quotaops.h |  8 ++++---
>  10 files changed, 93 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 2dcbd5698884..a13ba5dcb355 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1656,7 +1656,7 @@ int ext2_setattr(struct dentry *dentry, struct iattr *iattr)
>  	}
>  	if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) ||
>  	    (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) {
> -		error = dquot_transfer(inode, iattr);
> +		error = dquot_transfer(inode, iattr, 0);
>  		if (error)
>  			return error;
>  	}
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 6f5872197d6c..28abbbdbbb80 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5267,6 +5267,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
>  	int error, rc = 0;
>  	int orphan = 0;
>  	const unsigned int ia_valid = attr->ia_valid;
> +	int ea_inode_refs;
>  
>  	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
>  		return -EIO;
> @@ -5293,7 +5294,12 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
>  			error = PTR_ERR(handle);
>  			goto err_out;
>  		}
> -		error = dquot_transfer(inode, attr);
> +
> +		down_read(&EXT4_I(inode)->xattr_sem);
> +		error = ea_inode_refs = ext4_xattr_inode_count(inode);
> +		if (ea_inode_refs >= 0)
> +			error = dquot_transfer(inode, attr, ea_inode_refs);
> +		up_read(&EXT4_I(inode)->xattr_sem);
>  		if (error) {
>  			ext4_journal_stop(handle);
>  			return error;
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index dde8deb11e59..9938dc8e24c8 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -21,6 +21,7 @@
>  #include "ext4.h"
>  #include <linux/fsmap.h>
>  #include "fsmap.h"
> +#include "xattr.h"
>  #include <trace/events/ext4.h>
>  
>  /**
> @@ -319,6 +320,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>  	struct ext4_iloc iloc;
>  	struct ext4_inode *raw_inode;
>  	struct dquot *transfer_to[MAXQUOTAS] = { };
> +	int ea_inode_refs;
>  
>  	if (!ext4_has_feature_project(sb)) {
>  		if (projid != EXT4_DEF_PROJID)
> @@ -371,9 +373,17 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>  	if (err)
>  		goto out_stop;
>  
> +	down_read(&EXT4_I(inode)->xattr_sem);
> +	ea_inode_refs = ext4_xattr_inode_count(inode);
> +	if (ea_inode_refs < 0) {
> +		up_read(&EXT4_I(inode)->xattr_sem);
> +		err = ea_inode_refs;
> +		goto out_stop;
> +	}
> +
>  	transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid));
>  	if (!IS_ERR(transfer_to[PRJQUOTA])) {
> -		err = __dquot_transfer(inode, transfer_to);
> +		err = __dquot_transfer(inode, transfer_to, ea_inode_refs);
>  		dqput(transfer_to[PRJQUOTA]);
>  		if (err)
>  			goto out_dirty;
> @@ -382,6 +392,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>  	EXT4_I(inode)->i_projid = kprojid;
>  	inode->i_ctime = current_time(inode);
>  out_dirty:
> +	up_read(&EXT4_I(inode)->xattr_sem);
>  	rc = ext4_mark_iloc_dirty(handle, inode, &iloc);
>  	if (!err)
>  		err = rc;
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index caddc176a612..1d6fcbb01517 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -701,6 +701,60 @@ static void ext4_xattr_update_super_block(handle_t *handle,
>  	}
>  }
>  
> +int ext4_xattr_inode_count(struct inode *inode)
> +{
> +	struct ext4_iloc iloc = { .bh = NULL };
> +	struct buffer_head *bh = NULL;
> +	struct ext4_inode *raw_inode;
> +	struct ext4_xattr_ibody_header *header;
> +	struct ext4_xattr_entry *entry;
> +	int inode_count = 0;
> +	void *end;
> +	int ret;
> +
> +	lockdep_assert_held_read(&EXT4_I(inode)->xattr_sem);
> +
> +	if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
> +		ret = ext4_get_inode_loc(inode, &iloc);
> +		if (ret)
> +			goto out;
> +		raw_inode = ext4_raw_inode(&iloc);
> +		header = IHDR(inode, raw_inode);
> +		end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> +		ret = xattr_check_inode(inode, header, end);
> +		if (ret)
> +			goto out;
> +
> +		for (entry = IFIRST(header); !IS_LAST_ENTRY(entry);
> +		     entry = EXT4_XATTR_NEXT(entry))
> +			if (entry->e_value_inum)
> +				inode_count++;
> +	}
> +
> +	if (EXT4_I(inode)->i_file_acl) {
> +		bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
> +		if (!bh) {
> +			ret = -EIO;
> +			goto out;
> +		}
> +
> +		if (ext4_xattr_check_block(inode, bh)) {
> +			ret = -EFSCORRUPTED;
> +			goto out;
> +		}
> +
> +		for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry);
> +		     entry = EXT4_XATTR_NEXT(entry))
> +			if (entry->e_value_inum)
> +				inode_count++;
> +	}
> +	ret = inode_count;
> +out:
> +	brelse(iloc.bh);
> +	brelse(bh);
> +	return ret;
> +}
> +
>  static inline size_t round_up_cluster(struct inode *inode, size_t length)
>  {
>  	struct super_block *sb = inode->i_sb;
> diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
> index 67616cb9a059..8ef6fe123255 100644
> --- a/fs/ext4/xattr.h
> +++ b/fs/ext4/xattr.h
> @@ -193,3 +193,5 @@ extern void ext4_xattr_inode_set_class(struct inode *ea_inode);
>  #else
>  static inline void ext4_xattr_inode_set_class(struct inode *ea_inode) { }
>  #endif
> +
> +int ext4_xattr_inode_count(struct inode *inode);
> diff --git a/fs/jfs/file.c b/fs/jfs/file.c
> index 739492c7a3fd..b08e0b0449a7 100644
> --- a/fs/jfs/file.c
> +++ b/fs/jfs/file.c
> @@ -114,7 +114,7 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr)
>  	}
>  	if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) ||
>  	    (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) {
> -		rc = dquot_transfer(inode, iattr);
> +		rc = dquot_transfer(inode, iattr, 0);
>  		if (rc)
>  			return rc;
>  	}
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index bfeb647459d9..d3cbf6467af6 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1259,7 +1259,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
>  			mlog_errno(status);
>  			goto bail_unlock;
>  		}
> -		status = __dquot_transfer(inode, transfer_to);
> +		status = __dquot_transfer(inode, transfer_to, 0);
>  		if (status < 0)
>  			goto bail_commit;
>  	} else {
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 48813aeaab80..16e13d554aaa 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1906,10 +1906,12 @@ EXPORT_SYMBOL(dquot_free_inode);
>   * We are holding reference on transfer_from & transfer_to, no need to
>   * protect them by srcu_read_lock().
>   */
> -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to,
> +		     int inodes_extra)
>  {
>  	qsize_t space, cur_space;
>  	qsize_t rsv_space = 0;
> +	qsize_t inode_count = 1 + inodes_extra;
>  	struct dquot *transfer_from[MAXQUOTAS] = {};
>  	int cnt, ret = 0;
>  	char is_valid[MAXQUOTAS] = {};
> @@ -1946,7 +1948,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
>  			continue;
>  		is_valid[cnt] = 1;
>  		transfer_from[cnt] = i_dquot(inode)[cnt];
> -		ret = check_idq(transfer_to[cnt], 1, &warn_to[cnt]);
> +		ret = check_idq(transfer_to[cnt], inode_count, &warn_to[cnt]);
>  		if (ret)
>  			goto over_quota;
>  		ret = check_bdq(transfer_to[cnt], space, 0, &warn_to[cnt]);
> @@ -1963,7 +1965,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
>  		/* Due to IO error we might not have transfer_from[] structure */
>  		if (transfer_from[cnt]) {
>  			int wtype;
> -			wtype = info_idq_free(transfer_from[cnt], 1);
> +			wtype = info_idq_free(transfer_from[cnt], inode_count);
>  			if (wtype != QUOTA_NL_NOWARN)
>  				prepare_warning(&warn_from_inodes[cnt],
>  						transfer_from[cnt], wtype);
> @@ -1971,13 +1973,13 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
>  			if (wtype != QUOTA_NL_NOWARN)
>  				prepare_warning(&warn_from_space[cnt],
>  						transfer_from[cnt], wtype);
> -			dquot_decr_inodes(transfer_from[cnt], 1);
> +			dquot_decr_inodes(transfer_from[cnt], inode_count);
>  			dquot_decr_space(transfer_from[cnt], cur_space);
>  			dquot_free_reserved_space(transfer_from[cnt],
>  						  rsv_space);
>  		}
>  
> -		dquot_incr_inodes(transfer_to[cnt], 1);
> +		dquot_incr_inodes(transfer_to[cnt], inode_count);
>  		dquot_incr_space(transfer_to[cnt], cur_space);
>  		dquot_resv_space(transfer_to[cnt], rsv_space);
>  
> @@ -2005,7 +2007,7 @@ EXPORT_SYMBOL(__dquot_transfer);
>  /* Wrapper for transferring ownership of an inode for uid/gid only
>   * Called from FSXXX_setattr()
>   */
> -int dquot_transfer(struct inode *inode, struct iattr *iattr)
> +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra)
>  {
>  	struct dquot *transfer_to[MAXQUOTAS] = {};
>  	struct dquot *dquot;
> @@ -2037,7 +2039,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
>  		}
>  		transfer_to[GRPQUOTA] = dquot;
>  	}
> -	ret = __dquot_transfer(inode, transfer_to);
> +	ret = __dquot_transfer(inode, transfer_to, inodes_extra);
>  out_put:
>  	dqput_all(transfer_to);
>  	return ret;
> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index 873fc04e9403..51586051b5dd 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -3370,7 +3370,7 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)
>  		reiserfs_write_unlock(inode->i_sb);
>  		if (error)
>  			goto out;
> -		error = dquot_transfer(inode, attr);
> +		error = dquot_transfer(inode, attr, 0);
>  		reiserfs_write_lock(inode->i_sb);
>  		if (error) {
>  			journal_end(&th);
> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> index dda22f45fc1b..b7bcd9c6db6c 100644
> --- a/include/linux/quotaops.h
> +++ b/include/linux/quotaops.h
> @@ -106,8 +106,9 @@ int dquot_get_next_dqblk(struct super_block *sb, struct kqid *id,
>  int dquot_set_dqblk(struct super_block *sb, struct kqid id,
>  		struct qc_dqblk *di);
>  
> -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to);
> -int dquot_transfer(struct inode *inode, struct iattr *iattr);
> +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to,
> +		int inodes_extra);
> +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra);
>  
>  static inline struct mem_dqinfo *sb_dqinfo(struct super_block *sb, int type)
>  {
> @@ -226,7 +227,8 @@ static inline void dquot_free_inode(struct inode *inode)
>  {
>  }
>  
> -static inline int dquot_transfer(struct inode *inode, struct iattr *iattr)
> +static inline int dquot_transfer(struct inode *inode, struct iattr *iattr,
> +		int inodes_extra)
>  {
>  	return 0;
>  }
> -- 
> 2.13.0.219.gdb65acc882-goog
> 
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ