Re: [PATCH] quota: additional range checks and mem_dqblk updates to handle 64-bit limits

[Jan Kara <jack@suse.cz>]

On Fri 07-03-08 03:29:29, Andrew Perepechko wrote:
> mem_dqblk is extended to hold 64-bit quota limits, checks are added to deny
> setting quota limits which aren't supported by a quota format module
> 
> Signed-off-by: Andrew Perepechko <andrew.perepechko@sun.com>
  Great, thanks. The patch is fine. Yesterday evening I got an idea, how to
solve your problem with too low limits even easier. What we could do is to
introduce a "block-limit-scale" and "inode-limit-scale" parameter to the
quota info and we keep the rest of the file format the same. Now, the meaning
of this parameter would simply be a unit in which space and inode limits
are specified. When you have a filesystem where you'd like to set quotas
over 4 TB, you probably don't want to specify limits with 1KB precision
anyway... So you can just set scale to 1MB or even 16MB (giving you maximal
limit of 64 PB) and 10000 files or so. This has two advantages - only a few
trivial modifications to current kernel code, no change in quota file space
usage. We could then provide a way to set this scale via setquota / edquota
(which would have to convert the whole file but that should be no big deal).
  What do you think about such solution? Would it fit your needs? Sorry,
that I haven't through of this solution earlier...
								Honza
> 
> ---
> 
>  fs/dquot.c            |   22 +++++++++++++++++-----
>  fs/quota_v1.c         |    5 +++++
>  fs/quota_v2.c         |   15 ++++++++++-----
>  include/linux/quota.h |   14 +++++++++-----
>  4 files changed, 41 insertions(+), 15 deletions(-)
> 
> diff -rNpu quota.orig/fs/dquot.c quota/fs/dquot.c
> --- quota.orig/fs/dquot.c	2008-01-25 01:58:37.000000000 +0300
> +++ quota/fs/dquot.c	2008-03-07 01:37:46.000000000 +0300
> @@ -1703,10 +1703,19 @@ int vfs_get_dqblk(struct super_block *sb
>  }
>  
>  /* Generic routine for setting common part of quota structure */
> -static void do_set_dqblk(struct dquot *dquot, struct if_dqblk *di)
> +static int do_set_dqblk(struct dquot *dquot, struct if_dqblk *di)
>  {
>  	struct mem_dqblk *dm = &dquot->dq_dqb;
>  	int check_blim = 0, check_ilim = 0;
> +	struct mem_dqinfo *dqi = &sb_dqopt(dquot->dq_sb)->info[dquot->dq_type];
> +
> +	if (((di->dqb_valid & QIF_BLIMITS) &&
> +	    ((di->dqb_bhardlimit > dqi->dqi_maxbhardlimit) ||
> +	     (di->dqb_bsoftlimit > dqi->dqi_maxbsoftlimit))) ||
> +	    ((di->dqb_valid & QIF_ILIMITS) &&
> +	    ((di->dqb_ihardlimit > dqi->dqi_maxihardlimit) ||
> +	     (di->dqb_isoftlimit > dqi->dqi_maxisoftlimit))))
> +		return -ERANGE;
>  
>  	spin_lock(&dq_data_lock);
>  	if (di->dqb_valid & QIF_SPACE) {
> @@ -1738,7 +1747,7 @@ static void do_set_dqblk(struct dquot *d
>  			clear_bit(DQ_BLKS_B, &dquot->dq_flags);
>  		}
>  		else if (!(di->dqb_valid & QIF_BTIME))	/* Set grace only if user hasn't provided his own... */
> -			dm->dqb_btime = get_seconds() + sb_dqopt(dquot->dq_sb)->info[dquot->dq_type].dqi_bgrace;
> +			dm->dqb_btime = get_seconds() + dqi->dqi_bgrace;
>  	}
>  	if (check_ilim) {
>  		if (!dm->dqb_isoftlimit || dm->dqb_curinodes < dm->dqb_isoftlimit) {
> @@ -1746,7 +1755,7 @@ static void do_set_dqblk(struct dquot *d
>  			clear_bit(DQ_INODES_B, &dquot->dq_flags);
>  		}
>  		else if (!(di->dqb_valid & QIF_ITIME))	/* Set grace only if user hasn't provided his own... */
> -			dm->dqb_itime = get_seconds() + sb_dqopt(dquot->dq_sb)->info[dquot->dq_type].dqi_igrace;
> +			dm->dqb_itime = get_seconds() + dqi->dqi_igrace;
>  	}
>  	if (dm->dqb_bhardlimit || dm->dqb_bsoftlimit || dm->dqb_ihardlimit || dm->dqb_isoftlimit)
>  		clear_bit(DQ_FAKE_B, &dquot->dq_flags);
> @@ -1754,21 +1763,24 @@ static void do_set_dqblk(struct dquot *d
>  		set_bit(DQ_FAKE_B, &dquot->dq_flags);
>  	spin_unlock(&dq_data_lock);
>  	mark_dquot_dirty(dquot);
> +
> +	return 0;
>  }
>  
>  int vfs_set_dqblk(struct super_block *sb, int type, qid_t id, struct if_dqblk *di)
>  {
>  	struct dquot *dquot;
> +	int rc;
>  
>  	mutex_lock(&sb_dqopt(sb)->dqonoff_mutex);
>  	if (!(dquot = dqget(sb, id, type))) {
>  		mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex);
>  		return -ESRCH;
>  	}
> -	do_set_dqblk(dquot, di);
> +	rc = do_set_dqblk(dquot, di);
>  	dqput(dquot);
>  	mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex);
> -	return 0;
> +	return rc;
>  }
>  
>  /* Generic routine for getting common part of quota file information */
> diff -rNpu quota.orig/fs/quota_v1.c quota/fs/quota_v1.c
> --- quota.orig/fs/quota_v1.c	2008-01-25 01:58:37.000000000 +0300
> +++ quota/fs/quota_v1.c	2008-03-07 01:39:00.000000000 +0300
> @@ -139,6 +139,11 @@ static int v1_read_file_info(struct supe
>  		goto out;
>  	}
>  	ret = 0;
> +	/* limits are stored as unsigned 32-bit data */
> +	dqopt->info[type].dqi_maxbhardlimit = 0xffffffff;
> +	dqopt->info[type].dqi_maxbsoftlimit = 0xffffffff;
> +	dqopt->info[type].dqi_maxihardlimit = 0xffffffff;
> +	dqopt->info[type].dqi_maxisoftlimit = 0xffffffff;
>  	dqopt->info[type].dqi_igrace = dqblk.dqb_itime ? dqblk.dqb_itime : MAX_IQ_TIME;
>  	dqopt->info[type].dqi_bgrace = dqblk.dqb_btime ? dqblk.dqb_btime : MAX_DQ_TIME;
>  out:
> diff -rNpu quota.orig/fs/quota_v2.c quota/fs/quota_v2.c
> --- quota.orig/fs/quota_v2.c	2008-01-25 01:58:37.000000000 +0300
> +++ quota/fs/quota_v2.c	2008-03-07 02:40:23.000000000 +0300
> @@ -59,6 +59,11 @@ static int v2_read_file_info(struct supe
>  			sb->s_id);
>  		return -1;
>  	}
> +	/* limits are stored as unsigned 32-bit data */
> +	info->dqi_maxbhardlimit = 0xffffffff;
> +	info->dqi_maxbsoftlimit = 0xffffffff;
> +	info->dqi_maxihardlimit = 0xffffffff;
> +	info->dqi_maxisoftlimit = 0xffffffff;
>  	info->dqi_bgrace = le32_to_cpu(dinfo.dqi_bgrace);
>  	info->dqi_igrace = le32_to_cpu(dinfo.dqi_igrace);
>  	info->dqi_flags = le32_to_cpu(dinfo.dqi_flags);
> @@ -108,12 +113,12 @@ static void disk2memdqb(struct mem_dqblk
>  
>  static void mem2diskdqb(struct v2_disk_dqblk *d, struct mem_dqblk *m, qid_t id)
>  {
> -	d->dqb_ihardlimit = cpu_to_le32(m->dqb_ihardlimit);
> -	d->dqb_isoftlimit = cpu_to_le32(m->dqb_isoftlimit);
> -	d->dqb_curinodes = cpu_to_le32(m->dqb_curinodes);
> +	d->dqb_ihardlimit = cpu_to_le32((u32)m->dqb_ihardlimit);
> +	d->dqb_isoftlimit = cpu_to_le32((u32)m->dqb_isoftlimit);
> +	d->dqb_curinodes = cpu_to_le32((u32)m->dqb_curinodes);
>  	d->dqb_itime = cpu_to_le64(m->dqb_itime);
> -	d->dqb_bhardlimit = cpu_to_le32(m->dqb_bhardlimit);
> -	d->dqb_bsoftlimit = cpu_to_le32(m->dqb_bsoftlimit);
> +	d->dqb_bhardlimit = cpu_to_le32((u32)m->dqb_bhardlimit);
> +	d->dqb_bsoftlimit = cpu_to_le32((u32)m->dqb_bsoftlimit);
>  	d->dqb_curspace = cpu_to_le64(m->dqb_curspace);
>  	d->dqb_btime = cpu_to_le64(m->dqb_btime);
>  	d->dqb_id = cpu_to_le32(id);
> diff -rNpu quota.orig/include/linux/quota.h quota/include/linux/quota.h
> --- quota.orig/include/linux/quota.h	2008-01-25 01:58:37.000000000 +0300
> +++ quota/include/linux/quota.h	2008-03-06 23:18:10.000000000 +0300
> @@ -181,12 +181,12 @@ extern spinlock_t dq_data_lock;
>   * Data for one user/group kept in memory
>   */
>  struct mem_dqblk {
> -	__u32 dqb_bhardlimit;	/* absolute limit on disk blks alloc */
> -	__u32 dqb_bsoftlimit;	/* preferred limit on disk blks */
> +	qsize_t dqb_bhardlimit;	/* absolute limit on disk blks alloc */
> +	qsize_t dqb_bsoftlimit;	/* preferred limit on disk blks */
>  	qsize_t dqb_curspace;	/* current used space */
> -	__u32 dqb_ihardlimit;	/* absolute limit on allocated inodes */
> -	__u32 dqb_isoftlimit;	/* preferred inode limit */
> -	__u32 dqb_curinodes;	/* current # allocated inodes */
> +	qsize_t dqb_ihardlimit;	/* absolute limit on allocated inodes */
> +	qsize_t dqb_isoftlimit;	/* preferred inode limit */
> +	qsize_t dqb_curinodes;	/* current # allocated inodes */
>  	time_t dqb_btime;	/* time limit for excessive disk use */
>  	time_t dqb_itime;	/* time limit for excessive inode use */
>  };
> @@ -202,6 +202,10 @@ struct mem_dqinfo {
>  	unsigned long dqi_flags;
>  	unsigned int dqi_bgrace;
>  	unsigned int dqi_igrace;
> +	qsize_t dqi_maxbhardlimit;
> +	qsize_t dqi_maxbsoftlimit;
> +	qsize_t dqi_maxihardlimit;
> +	qsize_t dqi_maxisoftlimit;
>  	union {
>  		struct v1_mem_dqinfo v1_i;
>  		struct v2_mem_dqinfo v2_i;
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR