Re: [PATCH 5/6] xfs_repair: check dquot id and type

From: Chandan Babu R <chandanrlinux@gmail.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: sandeen@sandeen.net, linux-xfs@vger.kernel.org,
	Chaitanya.Kulkarni@wdc.com
Subject: Re: [PATCH 5/6] xfs_repair: check dquot id and type
Date: Wed, 10 Feb 2021 15:18:28 +0530	[thread overview]
Message-ID: <877dngp82b.fsf@garuda> (raw)
In-Reply-To: <161284390433.3058224.6853671538193339438.stgit@magnolia>

On 09 Feb 2021 at 09:41, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Make sure that we actually check the type and id of an ondisk dquot.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  repair/quotacheck.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 3 deletions(-)
>
>
> diff --git a/repair/quotacheck.c b/repair/quotacheck.c
> index 55bcc048..0ea2deb5 100644
> --- a/repair/quotacheck.c
> +++ b/repair/quotacheck.c
> @@ -234,12 +234,48 @@ quotacheck_adjust(
>  	libxfs_irele(ip);
>  }
>  
> +/* Check the ondisk dquot's id and type match what the incore dquot expects. */
> +static bool
> +qc_dquot_check_type(
> +	struct xfs_mount	*mp,
> +	xfs_dqtype_t		type,
> +	xfs_dqid_t		id,
> +	struct xfs_disk_dquot	*ddq)
> +{
> +	uint8_t			ddq_type;
> +
> +	ddq_type = ddq->d_type & XFS_DQTYPE_REC_MASK;
> +
> +	if (be32_to_cpu(ddq->d_id) != id)
> +		return false;
> +
> +	/*
> +	 * V5 filesystems always expect an exact type match.  V4 filesystems
> +	 * expect an exact match for user dquots and for non-root group and
> +	 * project dquots.
> +	 */
> +	if (xfs_sb_version_hascrc(&mp->m_sb) || type == XFS_DQTYPE_USER || !id)

The above check should have been "id" instead of "!id".

> +		return ddq_type == type;
> +
> +	/*
> +	 * V4 filesystems support either group or project quotas, but not both
> +	 * at the same time.  The non-user quota file can be switched between
> +	 * group and project quota uses depending on the mount options, which
> +	 * means that we can encounter the other type when we try to load quota
> +	 * defaults.  Quotacheck will soon reset the the entire quota file
> +	 * (including the root dquot) anyway, but don't log scary corruption
> +	 * reports to dmesg.
> +	 */
> +	return ddq_type == XFS_DQTYPE_GROUP || ddq_type == XFS_DQTYPE_PROJ;
> +}
> +
>  /* Compare this on-disk dquot against whatever we observed. */
>  static void
>  qc_check_dquot(
>  	struct xfs_mount	*mp,
>  	struct xfs_disk_dquot	*ddq,
> -	struct qc_dquots	*dquots)
> +	struct qc_dquots	*dquots,
> +	xfs_dqid_t		dqid)
>  {
>  	struct qc_rec		*qrec;
>  	struct qc_rec		empty = {
> @@ -253,6 +289,22 @@ qc_check_dquot(
>  	if (!qrec)
>  		qrec = &empty;
>  
> +	if (!qc_dquot_check_type(mp, dquots->type, dqid, ddq)) {
> +		const char	*dqtypestr;
> +
> +		dqtypestr = qflags_typestr(ddq->d_type & XFS_DQTYPE_REC_MASK);
> +		if (dqtypestr)
> +			do_warn(_("%s id %u saw type %s id %u\n"),
> +					qflags_typestr(dquots->type), dqid,
> +					dqtypestr, be32_to_cpu(ddq->d_id));
> +		else
> +			do_warn(_("%s id %u saw type %x id %u\n"),
> +					qflags_typestr(dquots->type), dqid,
> +					ddq->d_type & XFS_DQTYPE_REC_MASK,
> +					be32_to_cpu(ddq->d_id));
> +		chkd_flags = 0;
> +	}
> +
>  	if (be64_to_cpu(ddq->d_bcount) != qrec->bcount) {
>  		do_warn(_("%s id %u has bcount %llu, expected %"PRIu64"\n"),
>  				qflags_typestr(dquots->type), id,
> @@ -327,11 +379,11 @@ _("cannot read %s inode %"PRIu64", block %"PRIu64", disk block %"PRIu64", err=%d
>  		}
>  
>  		dqb = bp->b_addr;
> -		dqid = map->br_startoff * dqperchunk;
> +		dqid = (map->br_startoff + bno) * dqperchunk;
>  		for (dqnr = 0;
>  		     dqnr < dqperchunk && dqid <= UINT_MAX;
>  		     dqnr++, dqb++, dqid++)
> -			qc_check_dquot(mp, &dqb->dd_diskdq, dquots);
> +			qc_check_dquot(mp, &dqb->dd_diskdq, dquots, dqid);
>  		libxfs_buf_relse(bp);
>  	}
>  

-- 
chandan