From 050f9a08866fe29674348611b4fadb3c1f7d62c7 Mon Sep 17 00:00:00 2001 From: Brian Foster Date: Fri, 28 Nov 2014 14:00:53 +1100 Subject: [PATCH] xfs: fix error handling in xfs_qm_log_quotaoff() The error handling in xfs_qm_log_quotaoff() has a couple problems. If xfs_trans_commit() fails, we fall through to the error block and call xfs_trans_cancel(). This is incorrect on commit failure. If xfs_trans_reserve() fails, we jump to the error block, cancel the tp and restore the superblock qflags to oldsbqflag. However, oldsbqflag has been initialized to zero and not yet updated from the original flags so we set the flags to zero. Fix up the error handling in xfs_qm_log_quotaoff() to not restore flags if they haven't been modified and not cancel the tp on commit failure. Remove the flag restore code altogether because commit error is the only failure condition and we don't know whether the transaction made it to disk. Reported-by: Dan Carpenter Signed-off-by: Brian Foster Reviewed-by: Dave Chinner Signed-off-by: Dave Chinner --- fs/xfs/xfs_qm_syscalls.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c index 80f2d77d929a8..d1e0ab7a5d126 100644 --- a/fs/xfs/xfs_qm_syscalls.c +++ b/fs/xfs/xfs_qm_syscalls.c @@ -784,19 +784,21 @@ xfs_qm_log_quotaoff( { xfs_trans_t *tp; int error; - xfs_qoff_logitem_t *qoffi=NULL; - uint oldsbqflag=0; + xfs_qoff_logitem_t *qoffi; + + *qoffstartp = NULL; tp = xfs_trans_alloc(mp, XFS_TRANS_QM_QUOTAOFF); error = xfs_trans_reserve(tp, &M_RES(mp)->tr_qm_quotaoff, 0, 0); - if (error) - goto error0; + if (error) { + xfs_trans_cancel(tp, 0); + goto out; + } qoffi = xfs_trans_get_qoff_item(tp, NULL, flags & XFS_ALL_QUOTA_ACCT); xfs_trans_log_quotaoff_item(tp, qoffi); spin_lock(&mp->m_sb_lock); - oldsbqflag = mp->m_sb.sb_qflags; mp->m_sb.sb_qflags = (mp->m_qflags & ~(flags)) & XFS_MOUNT_QUOTA_ALL; spin_unlock(&mp->m_sb_lock); @@ -809,19 +811,11 @@ xfs_qm_log_quotaoff( */ xfs_trans_set_sync(tp); error = xfs_trans_commit(tp, 0); + if (error) + goto out; -error0: - if (error) { - xfs_trans_cancel(tp, 0); - /* - * No one else is modifying sb_qflags, so this is OK. - * We still hold the quotaofflock. - */ - spin_lock(&mp->m_sb_lock); - mp->m_sb.sb_qflags = oldsbqflag; - spin_unlock(&mp->m_sb_lock); - } *qoffstartp = qoffi; +out: return error; } -- 2.39.5