Re: [PATCH 03/16] tmpfs: preserve explicitly disabled quota limits

[Jan Kara]

On Mon 26-10-26 10:40:36, David Hildenbrand (Arm) wrote:
> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> 
> An administrator’s decision to disable a user’s or group’s quota
> limits can be silently lost when tmpfs restores the mount defaults
> after releasing and reacquiring the quota record. This can cause
> later file creation or writes to fail with EDQUOT, even though the
> administrator has explicitly disabled quota limits for the user or
> group.
> 
> do_set_dqblk() sets DQ_FAKE_B when all block and inode limits are zero.
> This means quota enforcement is disabled for that user or group.
> shmem_is_empty_dquot() treats the flag as a reason to discard the entry
> from the mount's quota tree. The next shmem_acquire_dquot() finds no
> saved entry and recreates it with the mount's default hard limits.
> 
> If the cached dquot is reused, DQ_FAKE_B remains set: Q_GETQUOTA reports
> the restored defaults, but enforcement is still disabled. Once that
> dquot has been reclaimed, a new dquot enforces the restored defaults.
> 
> Remove the DQ_FAKE_B test. Only discard entries with zero usage, zero
> soft limits and hard limits equal to the mount defaults. Explicitly
> disabled limits are then preserved whenever the mount defaults are
> nonzero.
> 
> Explicitly disabled limits now retain a quota-tree entry, just like other
> limits that differ from the mount defaults.
> 
> Rename shmem_is_empty_dquot() to shmem_is_default_dquot() to describe
> the check for default values.
> 
> Fix generated by an LLM and sent by Josef Bacik in private.
> 
> Fixes: de4c0e7ca8b5 ("shmem: Add default quota limit mount options")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> [ david: code cleanups; rewrite description ]
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Good catch. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  mm/shmem_quota.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/shmem_quota.c b/mm/shmem_quota.c
> index 43d3b4fd6cb1b..d191ebda3f00e 100644
> --- a/mm/shmem_quota.c
> +++ b/mm/shmem_quota.c
> @@ -239,7 +239,7 @@ static int shmem_acquire_dquot(struct dquot *dquot)
>  	return ret;
>  }
>  
> -static bool shmem_is_empty_dquot(struct dquot *dquot)
> +static bool shmem_is_default_dquot(struct dquot *dquot)
>  {
>  	struct shmem_sb_info *sbinfo = dquot->dq_sb->s_fs_info;
>  	qsize_t bhardlimit;
> @@ -253,21 +253,21 @@ static bool shmem_is_empty_dquot(struct dquot *dquot)
>  		ihardlimit = sbinfo->qlimits.grpquota_ihardlimit;
>  	}
>  
> -	if (test_bit(DQ_FAKE_B, &dquot->dq_flags) ||
> -		(dquot->dq_dqb.dqb_curspace == 0 &&
> -		 dquot->dq_dqb.dqb_curinodes == 0 &&
> -		 dquot->dq_dqb.dqb_bsoftlimit == 0 &&
> -		 dquot->dq_dqb.dqb_isoftlimit == 0 &&
> -		 dquot->dq_dqb.dqb_bhardlimit == bhardlimit &&
> -		 dquot->dq_dqb.dqb_ihardlimit == ihardlimit))
> -		return true;
> -
> -	return false;
> +	/*
> +	 * Defaults are zero usage and soft limits, and the mount's hard limits.
> +	 * DQ_FAKE_B only means that *all* limits are zero, which need not match
> +	 * the mount defaults.
> +	 */
> +	return dquot->dq_dqb.dqb_curspace == 0 &&
> +	       dquot->dq_dqb.dqb_curinodes == 0 &&
> +	       dquot->dq_dqb.dqb_bsoftlimit == 0 &&
> +	       dquot->dq_dqb.dqb_isoftlimit == 0 &&
> +	       dquot->dq_dqb.dqb_bhardlimit == bhardlimit &&
> +	       dquot->dq_dqb.dqb_ihardlimit == ihardlimit;
>  }
>  /*
> - * Store limits from dquot in the tree unless it's fake. If it is fake
> - * remove the id from the tree since there is no useful information in
> - * there.
> + * Remove the ID from the tree if it has no usage and its limits match
> + * the mount defaults. Otherwise, save its limits for the next acquire.
>   */
>  static int shmem_release_dquot(struct dquot *dquot)
>  {
> @@ -302,7 +302,7 @@ static int shmem_release_dquot(struct dquot *dquot)
>  	return -ENOENT;
>  
>  found:
> -	if (shmem_is_empty_dquot(dquot)) {
> +	if (shmem_is_default_dquot(dquot)) {
>  		/* Remove entry from the tree */
>  		rb_erase(&entry->node, info->dqi_priv);
>  		kfree(entry);
> 
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR