xfs
[Top] [All Lists]

[PATCH] xfs: fix error handling in xfs_qm_log_quotaoff()

To: xfs@xxxxxxxxxxx
Subject: [PATCH] xfs: fix error handling in xfs_qm_log_quotaoff()
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 16 Oct 2014 09:39:33 -0400
Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
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.

Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
---
 fs/xfs/xfs_qm_syscalls.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 80f2d77..f2d454e 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -784,13 +784,17 @@ xfs_qm_log_quotaoff(
 {
        xfs_trans_t            *tp;
        int                     error;
-       xfs_qoff_logitem_t     *qoffi=NULL;
-       uint                    oldsbqflag=0;
+       xfs_qoff_logitem_t     *qoffi;
+       uint                    oldsbqflag;
+
+       *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);
+               return error;
+       }
 
        qoffi = xfs_trans_get_qoff_item(tp, NULL, flags & XFS_ALL_QUOTA_ACCT);
        xfs_trans_log_quotaoff_item(tp, qoffi);
@@ -809,19 +813,20 @@ xfs_qm_log_quotaoff(
         */
        xfs_trans_set_sync(tp);
        error = xfs_trans_commit(tp, 0);
+       if (error)
+               goto error_flags;
 
-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;
+       return 0;
+
+error_flags:
+       /*
+        * 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);
        return error;
 }
 
-- 
1.8.3.1

<Prev in Thread] Current Thread [Next in Thread>