Re: [PATCH 4/5] xfs_repair: skip the rmap and refcount btree checks when the levels are garbage

Allison Henderson
2020-10-27T05:35:58Z

On 10/26/20 4:32 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> In validate_ag[fi], we should check that the levels of the rmap and
> refcount btrees are valid. If they aren't, we need to tell phase4 to
> skip the comparison between the existing and incore rmap and refcount
> data. The comparison routines use libxfs btree cursors, which assume
> that the caller validated bc_nlevels and will corrupt memory if we load
> a btree cursor with a garbage level count.
>
> This was found by examing a core dump from a failed xfs/086 invocation.
>
Ok, looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> repair/scan.c | 36 ++++++++++++++++++++++++++----------
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
>
> diff --git a/repair/scan.c b/repair/scan.c
> index 42b299f75067..2a38ae5197c6 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
> @@ -2279,23 +2279,31 @@ validate_agf(
>
> if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
> struct rmap_priv priv;
> + unsigned int levels;
>
> memset(&priv.high_key, 0xFF, sizeof(priv.high_key));
> priv.high_key.rm_blockcount = 0;
> priv.agcnts = agcnts;
> priv.last_rec.rm_owner = XFS_RMAP_OWN_UNKNOWN;
> priv.nr_blocks = 0;
> +
> + levels = be32_to_cpu(agf->agf_levels[XFS_BTNUM_RMAP]);
> + if (levels >= XFS_BTREE_MAXLEVELS) {
> + do_warn(_("bad levels %u for rmapbt root, agno %d\n"),
> + levels, agno);
> + rmap_avoid_check();
> + }
> +
> bno = be32_to_cpu(agf->agf_roots[XFS_BTNUM_RMAP]);
> if (libxfs_verify_agbno(mp, agno, bno)) {
> - scan_sbtree(bno,
> - be32_to_cpu(agf->agf_levels[XFS_BTNUM_RMAP]),
> - agno, 0, scan_rmapbt, 1, XFS_RMAP_CRC_MAGIC,
> - &priv, &xfs_rmapbt_buf_ops);
> + scan_sbtree(bno, levels, agno, 0, scan_rmapbt, 1,
> + XFS_RMAP_CRC_MAGIC, &priv,
> + &xfs_rmapbt_buf_ops);
> if (be32_to_cpu(agf->agf_rmap_blocks) != priv.nr_blocks)
> do_warn(_("bad rmapbt block count %u, saw %u\n"),
> priv.nr_blocks,
> be32_to_cpu(agf->agf_rmap_blocks));
> - } else {
> + } else {
> do_warn(_("bad agbno %u for rmapbt root, agno %d\n"),
> bno, agno);
> rmap_avoid_check();
> @@ -2303,20 +2311,28 @@ validate_agf(
> }
>
> if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> + unsigned int levels;
> +
> + levels = be32_to_cpu(agf->agf_refcount_level);
> + if (levels >= XFS_BTREE_MAXLEVELS) {
> + do_warn(_("bad levels %u for refcountbt root, agno %d\n"),
> + levels, agno);
> + refcount_avoid_check();
> + }
> +
> bno = be32_to_cpu(agf->agf_refcount_root);
> if (libxfs_verify_agbno(mp, agno, bno)) {
> struct refc_priv priv;
>
> memset(&priv, 0, sizeof(priv));
> - scan_sbtree(bno,
> - be32_to_cpu(agf->agf_refcount_level),
> - agno, 0, scan_refcbt, 1, XFS_REFC_CRC_MAGIC,
> - &priv, &xfs_refcountbt_buf_ops);
> + scan_sbtree(bno, levels, agno, 0, scan_refcbt, 1,
> + XFS_REFC_CRC_MAGIC, &priv,
> + &xfs_refcountbt_buf_ops);
> if (be32_to_cpu(agf->agf_refcount_blocks) != priv.nr_blocks)
> do_warn(_("bad refcountbt block count %u, saw %u\n"),
> priv.nr_blocks,
> be32_to_cpu(agf->agf_refcount_blocks));
> - } else {
> + } else {
> do_warn(_("bad agbno %u for refcntbt root, agno %d\n"),
> bno, agno);
> refcount_avoid_check();
>