xfs: ensure logflagsp is initialized in xfs_bmap_del_extent_real

In the case of returning -ENOSPC, ensure logflagsp is initialized by 0.
Otherwise the caller __xfs_bunmapi will set uninitialized illegal
tmp_logflags value into xfs log, which might cause unpredictable error
in the log recovery procedure.

Also, remove the flags variable and set the *logflagsp directly, so that
the code should be more robust in the long run.

Fixes: 1b24b633aafe ("xfs: move some more code into xfs_bmap_del_extent_real")
Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 68be1dd4f0f268602c7929fac0a701fd2d95cd77..ca6614f4eac50a072a4c7aeb0086370abf96b366 100644 (file)
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5010,7 +5010,6 @@ xfs_bmap_del_extent_real(
        xfs_fileoff_t           del_endoff;     /* first offset past del */
        int                     do_fx;  /* free extent at end of routine */
        int                     error;  /* error return value */
-       int                     flags = 0;/* inode logging flags */
        struct xfs_bmbt_irec    got;    /* current extent entry */
        xfs_fileoff_t           got_endoff;     /* first offset past got */
        int                     i;      /* temp state */
@@ -5023,6 +5022,8 @@ xfs_bmap_del_extent_real(
        uint32_t                state = xfs_bmap_fork_to_state(whichfork);
        struct xfs_bmbt_irec    old;
 
+       *logflagsp = 0;
+
        mp = ip->i_mount;
        XFS_STATS_INC(mp, xs_del_exlist);
 
@@ -5035,7 +5036,6 @@ xfs_bmap_del_extent_real(
        ASSERT(got_endoff >= del_endoff);
        ASSERT(!isnullstartblock(got.br_startblock));
        qfield = 0;
-       error = 0;
 
        /*
         * If it's the case where the directory code is running with no block
@@ -5051,13 +5051,13 @@ xfs_bmap_del_extent_real(
            del->br_startoff > got.br_startoff && del_endoff < got_endoff)
                return -ENOSPC;
 
-       flags = XFS_ILOG_CORE;
+       *logflagsp = XFS_ILOG_CORE;
        if (whichfork == XFS_DATA_FORK && XFS_IS_REALTIME_INODE(ip)) {
                if (!(bflags & XFS_BMAPI_REMAP)) {
                        error = xfs_rtfree_blocks(tp, del->br_startblock,
                                        del->br_blockcount);
                        if (error)
-                               goto done;
+                               return error;
                }
 
                do_fx = 0;
@@ -5072,11 +5072,9 @@ xfs_bmap_del_extent_real(
        if (cur) {
                error = xfs_bmbt_lookup_eq(cur, &got, &i);
                if (error)
-                       goto done;
-               if (XFS_IS_CORRUPT(mp, i != 1)) {
-                       error = -EFSCORRUPTED;
-                       goto done;
-               }
+                       return error;
+               if (XFS_IS_CORRUPT(mp, i != 1))
+                       return -EFSCORRUPTED;
        }
 
        if (got.br_startoff == del->br_startoff)
@@ -5093,17 +5091,15 @@ xfs_bmap_del_extent_real(
                xfs_iext_prev(ifp, icur);
                ifp->if_nextents--;
 
-               flags |= XFS_ILOG_CORE;
+               *logflagsp |= XFS_ILOG_CORE;
                if (!cur) {
-                       flags |= xfs_ilog_fext(whichfork);
+                       *logflagsp |= xfs_ilog_fext(whichfork);
                        break;
                }
                if ((error = xfs_btree_delete(cur, &i)))
-                       goto done;
-               if (XFS_IS_CORRUPT(mp, i != 1)) {
-                       error = -EFSCORRUPTED;
-                       goto done;
-               }
+                       return error;
+               if (XFS_IS_CORRUPT(mp, i != 1))
+                       return -EFSCORRUPTED;
                break;
        case BMAP_LEFT_FILLING:
                /*
@@ -5114,12 +5110,12 @@ xfs_bmap_del_extent_real(
                got.br_blockcount -= del->br_blockcount;
                xfs_iext_update_extent(ip, state, icur, &got);
                if (!cur) {
-                       flags |= xfs_ilog_fext(whichfork);
+                       *logflagsp |= xfs_ilog_fext(whichfork);
                        break;
                }
                error = xfs_bmbt_update(cur, &got);
                if (error)
-                       goto done;
+                       return error;
                break;
        case BMAP_RIGHT_FILLING:
                /*
@@ -5128,12 +5124,12 @@ xfs_bmap_del_extent_real(
                got.br_blockcount -= del->br_blockcount;
                xfs_iext_update_extent(ip, state, icur, &got);
                if (!cur) {
-                       flags |= xfs_ilog_fext(whichfork);
+                       *logflagsp |= xfs_ilog_fext(whichfork);
                        break;
                }
                error = xfs_bmbt_update(cur, &got);
                if (error)
-                       goto done;
+                       return error;
                break;
        case 0:
                /*
@@ -5150,18 +5146,18 @@ xfs_bmap_del_extent_real(
                new.br_state = got.br_state;
                new.br_startblock = del_endblock;
 
-               flags |= XFS_ILOG_CORE;
+               *logflagsp |= XFS_ILOG_CORE;
                if (cur) {
                        error = xfs_bmbt_update(cur, &got);
                        if (error)
-                               goto done;
+                               return error;
                        error = xfs_btree_increment(cur, 0, &i);
                        if (error)
-                               goto done;
+                               return error;
                        cur->bc_rec.b = new;
                        error = xfs_btree_insert(cur, &i);
                        if (error && error != -ENOSPC)
-                               goto done;
+                               return error;
                        /*
                         * If get no-space back from btree insert, it tried a
                         * split, and we have a zero block reservation.  Fix up
@@ -5174,33 +5170,28 @@ xfs_bmap_del_extent_real(
                                 */
                                error = xfs_bmbt_lookup_eq(cur, &got, &i);
                                if (error)
-                                       goto done;
-                               if (XFS_IS_CORRUPT(mp, i != 1)) {
-                                       error = -EFSCORRUPTED;
-                                       goto done;
-                               }
+                                       return error;
+                               if (XFS_IS_CORRUPT(mp, i != 1))
+                                       return -EFSCORRUPTED;
                                /*
                                 * Update the btree record back
                                 * to the original value.
                                 */
                                error = xfs_bmbt_update(cur, &old);
                                if (error)
-                                       goto done;
+                                       return error;
                                /*
                                 * Reset the extent record back
                                 * to the original value.
                                 */
                                xfs_iext_update_extent(ip, state, icur, &old);
-                               flags = 0;
-                               error = -ENOSPC;
-                               goto done;
-                       }
-                       if (XFS_IS_CORRUPT(mp, i != 1)) {
-                               error = -EFSCORRUPTED;
-                               goto done;
+                               *logflagsp = 0;
+                               return -ENOSPC;
                        }
+                       if (XFS_IS_CORRUPT(mp, i != 1))
+                               return -EFSCORRUPTED;
                } else
-                       flags |= xfs_ilog_fext(whichfork);
+                       *logflagsp |= xfs_ilog_fext(whichfork);
 
                ifp->if_nextents++;
                xfs_iext_next(ifp, icur);
@@ -5224,7 +5215,7 @@ xfs_bmap_del_extent_real(
                                        ((bflags & XFS_BMAPI_NODISCARD) ||
                                        del->br_state == XFS_EXT_UNWRITTEN));
                        if (error)
-                               goto done;
+                               return error;
                }
        }
 
@@ -5239,9 +5230,7 @@ xfs_bmap_del_extent_real(
        if (qfield && !(bflags & XFS_BMAPI_REMAP))
                xfs_trans_mod_dquot_byino(tp, ip, qfield, (long)-nblks);
 
-done:
-       *logflagsp = flags;
-       return error;
+       return 0;
 }
 
 /*