[bug report] xfs: don't swallow dquot recovery verification errors

[Dan Carpenter <error27@gmail.com>]

Hello Long Li,

Commit e2b4a856085e ("xfs: don't swallow dquot recovery verification
errors") from Jul 27, 2026 (linux-next), leads to the following
Smatch static checker warning:

	fs/xfs/xfs_dquot_item_recover.c:146 xlog_recover_dquot_commit_pass2()
	warn: missing error code here? 'xfs_dquot_verify()' failed. 'error' = '0'

fs/xfs/xfs_dquot_item_recover.c
    60 STATIC int
    61 xlog_recover_dquot_commit_pass2(
    62         struct xlog                        *log,
    63         struct list_head                *buffer_list,
    64         struct xlog_recover_item        *item,
    65         xfs_lsn_t                        current_lsn)
    66 {
    67         struct xfs_mount                *mp = log->l_mp;
    68         struct xfs_buf                        *bp;
    69         struct xfs_dqblk                *dqb;
    70         struct xfs_disk_dquot                *ddq, *recddq;
    71         struct xfs_dq_logformat                *dq_f;
    72         xfs_failaddr_t                        fa;
    73         int                                error;
    74         uint                                type;
    75 
    76         /*
    77          * Filesystems are required to send in quota flags at mount time.
    78          */
    79         if (mp->m_qflags == 0)
    80                 return 0;
    81 
    82         recddq = item->ri_buf[1].iov_base;
    83         if (recddq == NULL) {
    84                 xfs_alert(log->l_mp, "NULL dquot in %s.", __func__);
    85                 return -EFSCORRUPTED;
    86         }
    87         if (item->ri_buf[1].iov_len < sizeof(struct xfs_disk_dquot)) {
    88                 xfs_alert(log->l_mp, "dquot too small (%zd) in %s.",
    89                         item->ri_buf[1].iov_len, __func__);
    90                 return -EFSCORRUPTED;
    91         }
    92 
    93         /*
    94          * This type of quotas was turned off, so ignore this record.
    95          */
    96         type = recddq->d_type & XFS_DQTYPE_REC_MASK;
    97         ASSERT(type);
    98         if (log->l_quotaoffs_flag & type)
    99                 return 0;
    100 
    101         /*
    102          * At this point we know that quota was _not_ turned off.
    103          * Since the mount flags are not indicating to us otherwise, this
    104          * must mean that quota is on, and the dquot needs to be replayed.
    105          * Remember that we may not have fully recovered the superblock yet,
    106          * so we can't do the usual trick of looking at the SB quota bits.
    107          *
    108          * The other possibility, of course, is that the quota subsystem was
    109          * removed since the last mount - ENOSYS.
    110          */
    111         dq_f = item->ri_buf[0].iov_base;
    112         ASSERT(dq_f);
    113         fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id);
    114         if (fa) {
    115                 xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
    116                                 dq_f->qlf_id, fa);
    117                 return -EFSCORRUPTED;
    118         }
    119         ASSERT(dq_f->qlf_len == 1);
    120 
    121         /*
    122          * At this point we are assuming that the dquots have been allocated
    123          * and hence the buffer has valid dquots stamped in it. It should,
    124          * therefore, pass verifier validation. If the dquot is bad, then the
    125          * we'll return an error here, so we don't need to specifically check
    126          * the dquot in the buffer after the verifier has run.
    127          */
    128         error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dq_f->qlf_blkno,
    129                                    XFS_FSB_TO_BB(mp, dq_f->qlf_len), 0, &bp,
    130                                    &xfs_dquot_buf_ops);
    131         if (error)
    132                 return error;
    133 
    134         ASSERT(bp);
    135         dqb = xfs_buf_offset(bp, dq_f->qlf_boffset);
    136         ddq = &dqb->dd_diskdq;
    137 
    138         /*
    139          * If the dquot has an LSN in it, recover the dquot only if it's less
    140          * than the lsn of the transaction we are replaying.
    141          */
    142         if (xfs_has_crc(mp)) {
    143                 xfs_lsn_t        lsn = be64_to_cpu(dqb->dd_lsn);
    144 
    145                 if (lsn && lsn != -1 && XFS_LSN_CMP(lsn, current_lsn) >= 0) {
--> 146                         goto out_release;

Originally this function always returned zero, but now it returns
error codes.  Should this be an error path?

    147                 }
    148         }
    149 
    150         memcpy(ddq, recddq, item->ri_buf[1].iov_len);
    151         if (xfs_has_crc(mp)) {
    152                 xfs_update_cksum((char *)dqb, sizeof(struct xfs_dqblk),
    153                                  XFS_DQUOT_CRC_OFF);
    154         }
    155 
    156         /* Validate the recovered dquot. */
    157         fa = xfs_dqblk_verify(log->l_mp, dqb, dq_f->qlf_id);
    158         if (fa) {
    159                 XFS_CORRUPTION_ERROR("Bad dquot after recovery",
    160                                 XFS_ERRLEVEL_LOW, mp, dqb,
    161                                 sizeof(struct xfs_dqblk));
    162                 xfs_alert(mp,
    163  "Metadata corruption detected at %pS, dquot 0x%x",
    164                                 fa, dq_f->qlf_id);
    165                 error = -EFSCORRUPTED;
    166                 goto out_release;
    167         }
    168 
    169         ASSERT(dq_f->qlf_size == 2);
    170         ASSERT(bp->b_mount == mp);
    171         xfs_buf_delwri_queue(bp, buffer_list);
    172 
    173 out_release:
    174         xfs_buf_relse(bp);
    175         return error;
    176 }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter