]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
xfs: fix error returns from xfs_bmapi_write
authorChristoph Hellwig <hch@lst.de>
Wed, 30 Apr 2025 21:26:48 +0000 (14:26 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 9 May 2025 07:41:36 +0000 (09:41 +0200)
[ Upstream commit 6773da870ab89123d1b513da63ed59e32a29cb77 ]

xfs_bmapi_write can return 0 without actually returning a mapping in
mval in two different cases:

 1) when there is absolutely no space available to do an allocation
 2) when converting delalloc space, and the allocation is so small
    that it only covers parts of the delalloc extent before the
    range requested by the caller

Callers at best can handle one of these cases, but in many cases can't
cope with either one.  Switch xfs_bmapi_write to always return a
mapping or return an error code instead.  For case 1) above ENOSPC is
the obvious choice which is very much what the callers expect anyway.
For case 2) there is no really good error code, so pick a funky one
from the SysV streams portfolio.

This fixes the reproducer here:

    https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/

which uses reserved blocks to create file systems that are gravely
out of space and thus cause at least xfs_file_alloc_space to hang
and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc.

Note that this patch does not actually make any caller but
xfs_alloc_file_space deal intelligently with case 2) above.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: 刘通 <lyutoon@gmail.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/xfs/libxfs/xfs_attr_remote.c
fs/xfs/libxfs/xfs_bmap.c
fs/xfs/libxfs/xfs_da_btree.c
fs/xfs/xfs_bmap_util.c
fs/xfs/xfs_dquot.c
fs/xfs/xfs_iomap.c
fs/xfs/xfs_reflink.c
fs/xfs/xfs_rtalloc.c

index d440393b40eb8e738856956f55f6bca41ce8e3a5..54de405cbab5ac922849e0719568c0effe334662 100644 (file)
@@ -619,7 +619,6 @@ xfs_attr_rmtval_set_blk(
        if (error)
                return error;
 
-       ASSERT(nmap == 1);
        ASSERT((map->br_startblock != DELAYSTARTBLOCK) &&
               (map->br_startblock != HOLESTARTBLOCK));
 
index 2a3b78032cb0c4b22a47c9a74d8cb7af915b84b8..0235c1dd3d7e1e98caa7b05fbc692c10a5580c17 100644 (file)
@@ -4113,8 +4113,10 @@ xfs_bmapi_allocate(
        } else {
                error = xfs_bmap_alloc_userdata(bma);
        }
-       if (error || bma->blkno == NULLFSBLOCK)
+       if (error)
                return error;
+       if (bma->blkno == NULLFSBLOCK)
+               return -ENOSPC;
 
        if (bma->flags & XFS_BMAPI_ZERO) {
                error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
@@ -4294,6 +4296,15 @@ xfs_bmapi_finish(
  * extent state if necessary.  Details behaviour is controlled by the flags
  * parameter.  Only allocates blocks from a single allocation group, to avoid
  * locking problems.
+ *
+ * Returns 0 on success and places the extent mappings in mval.  nmaps is used
+ * as an input/output parameter where the caller specifies the maximum number
+ * of mappings that may be returned and xfs_bmapi_write passes back the number
+ * of mappings (including existing mappings) it found.
+ *
+ * Returns a negative error code on failure, including -ENOSPC when it could not
+ * allocate any blocks and -ENOSR when it did allocate blocks to convert a
+ * delalloc range, but those blocks were before the passed in range.
  */
 int
 xfs_bmapi_write(
@@ -4421,10 +4432,16 @@ xfs_bmapi_write(
                        ASSERT(len > 0);
                        ASSERT(bma.length > 0);
                        error = xfs_bmapi_allocate(&bma);
-                       if (error)
+                       if (error) {
+                               /*
+                                * If we already allocated space in a previous
+                                * iteration return what we go so far when
+                                * running out of space.
+                                */
+                               if (error == -ENOSPC && bma.nallocs)
+                                       break;
                                goto error0;
-                       if (bma.blkno == NULLFSBLOCK)
-                               break;
+                       }
 
                        /*
                         * If this is a CoW allocation, record the data in
@@ -4462,7 +4479,6 @@ xfs_bmapi_write(
                if (!xfs_iext_next_extent(ifp, &bma.icur, &bma.got))
                        eof = true;
        }
-       *nmap = n;
 
        error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags,
                        whichfork);
@@ -4473,7 +4489,22 @@ xfs_bmapi_write(
               ifp->if_nextents > XFS_IFORK_MAXEXT(ip, whichfork));
        xfs_bmapi_finish(&bma, whichfork, 0);
        xfs_bmap_validate_ret(orig_bno, orig_len, orig_flags, orig_mval,
-               orig_nmap, *nmap);
+               orig_nmap, n);
+
+       /*
+        * When converting delayed allocations, xfs_bmapi_allocate ignores
+        * the passed in bno and always converts from the start of the found
+        * delalloc extent.
+        *
+        * To avoid a successful return with *nmap set to 0, return the magic
+        * -ENOSR error code for this particular case so that the caller can
+        * handle it.
+        */
+       if (!n) {
+               ASSERT(bma.nallocs >= *nmap);
+               return -ENOSR;
+       }
+       *nmap = n;
        return 0;
 error0:
        xfs_bmapi_finish(&bma, whichfork, error);
@@ -4580,9 +4611,6 @@ xfs_bmapi_convert_delalloc(
        if (error)
                goto out_finish;
 
-       error = -ENOSPC;
-       if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
-               goto out_finish;
        error = -EFSCORRUPTED;
        if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock)))
                goto out_finish;
index 282c7cf032f406b038f62fcce7a17d4f135e8fb1..12e3cca804b7ecdd2bbbb198242e199015896d57 100644 (file)
@@ -2158,8 +2158,8 @@ xfs_da_grow_inode_int(
        struct xfs_inode        *dp = args->dp;
        int                     w = args->whichfork;
        xfs_rfsblock_t          nblks = dp->i_nblocks;
-       struct xfs_bmbt_irec    map, *mapp;
-       int                     nmap, error, got, i, mapi;
+       struct xfs_bmbt_irec    map, *mapp = &map;
+       int                     nmap, error, got, i, mapi = 1;
 
        /*
         * Find a spot in the file space to put the new block.
@@ -2175,14 +2175,7 @@ xfs_da_grow_inode_int(
        error = xfs_bmapi_write(tp, dp, *bno, count,
                        xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
                        args->total, &map, &nmap);
-       if (error)
-               return error;
-
-       ASSERT(nmap <= 1);
-       if (nmap == 1) {
-               mapp = &map;
-               mapi = 1;
-       } else if (nmap == 0 && count > 1) {
+       if (error == -ENOSPC && count > 1) {
                xfs_fileoff_t           b;
                int                     c;
 
@@ -2199,16 +2192,13 @@ xfs_da_grow_inode_int(
                                        args->total, &mapp[mapi], &nmap);
                        if (error)
                                goto out_free_map;
-                       if (nmap < 1)
-                               break;
                        mapi += nmap;
                        b = mapp[mapi - 1].br_startoff +
                            mapp[mapi - 1].br_blockcount;
                }
-       } else {
-               mapi = 0;
-               mapp = NULL;
        }
+       if (error)
+               goto out_free_map;
 
        /*
         * Count the blocks we got, make sure it matches the total.
index 468bb61a5e46dfde076fba1b358642203e753201..399451aab26ab37ad750b74470646ca86909d8a6 100644 (file)
@@ -868,33 +868,32 @@ xfs_alloc_file_space(
                if (error)
                        goto error;
 
-               error = xfs_bmapi_write(tp, ip, startoffset_fsb,
-                               allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
-                               &nimaps);
-               if (error)
-                       goto error;
-
-               ip->i_diflags |= XFS_DIFLAG_PREALLOC;
-               xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-
-               error = xfs_trans_commit(tp);
-               xfs_iunlock(ip, XFS_ILOCK_EXCL);
-               if (error)
-                       break;
-
                /*
                 * If the allocator cannot find a single free extent large
                 * enough to cover the start block of the requested range,
-                * xfs_bmapi_write will return 0 but leave *nimaps set to 0.
+                * xfs_bmapi_write will return -ENOSR.
                 *
                 * In that case we simply need to keep looping with the same
                 * startoffset_fsb so that one of the following allocations
                 * will eventually reach the requested range.
                 */
-               if (nimaps) {
+               error = xfs_bmapi_write(tp, ip, startoffset_fsb,
+                               allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
+                               &nimaps);
+               if (error) {
+                       if (error != -ENOSR)
+                               goto error;
+                       error = 0;
+               } else {
                        startoffset_fsb += imapp->br_blockcount;
                        allocatesize_fsb -= imapp->br_blockcount;
                }
+
+               ip->i_diflags |= XFS_DIFLAG_PREALLOC;
+               xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+               error = xfs_trans_commit(tp);
+               xfs_iunlock(ip, XFS_ILOCK_EXCL);
        }
 
        return error;
index a8b2f3b278ea5eb0b94cddaa4420574e0900caf9..6186b69be50a1539c4bbfd09ee3761ffb3f462ae 100644 (file)
@@ -333,7 +333,6 @@ xfs_dquot_disk_alloc(
                goto err_cancel;
 
        ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
-       ASSERT(nmaps == 1);
        ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
               (map.br_startblock != HOLESTARTBLOCK));
 
index ab5512c0bcf7a91db02f62401d7184161cb52e53..60d9638cec6dabda67b04ddd078eaea8a6a34ecd 100644 (file)
@@ -309,14 +309,6 @@ xfs_iomap_write_direct(
        if (error)
                goto out_unlock;
 
-       /*
-        * Copy any maps to caller's array and return any error.
-        */
-       if (nimaps == 0) {
-               error = -ENOSPC;
-               goto out_unlock;
-       }
-
        if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock)))
                error = xfs_alert_fsblock_zero(ip, imap);
 
index 1bac6a8af970531b01a649b5246827a4ea791276..0405226fb74c8af51059e7bc3f7006a0d385f071 100644 (file)
@@ -431,13 +431,6 @@ xfs_reflink_fill_cow_hole(
        if (error)
                return error;
 
-       /*
-        * Allocation succeeded but the requested range was not even partially
-        * satisfied?  Bail out!
-        */
-       if (nimaps == 0)
-               return -ENOSPC;
-
 convert:
        return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
 
@@ -500,13 +493,6 @@ xfs_reflink_fill_delalloc(
                error = xfs_trans_commit(tp);
                if (error)
                        return error;
-
-               /*
-                * Allocation succeeded but the requested range was not even
-                * partially satisfied?  Bail out!
-                */
-               if (nimaps == 0)
-                       return -ENOSPC;
        } while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
 
        return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
index 7c5134899634b7991bda401b24f9c3e73eafe428..5cf1e91f4c205e3a20e11c0ed6a08953a782993e 100644 (file)
@@ -840,8 +840,6 @@ xfs_growfs_rt_alloc(
                nmap = 1;
                error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks,
                                        XFS_BMAPI_METADATA, 0, &map, &nmap);
-               if (!error && nmap < 1)
-                       error = -ENOSPC;
                if (error)
                        goto out_trans_cancel;
                /*