xfs: unmapped buffer item size straddling mismatch

We never log large contiguous regions of unmapped buffers, so this
bug is never triggered by the current code. However, the slowpath
for formatting buffer straddling regions is broken.

That is, the size and shape of the log vector calculated across a
straddle does not match how the formatting code formats a straddle.
This results in a log vector with an uninitialised iovec and this
causes a crash when xlog_write_full() goes to copy the iovec into
the journal.

Whilst touching this code, don't bother checking mapped or single
folio buffers for discontiguous regions because they don't have
them. This significantly reduces the overhead of this check when
logging large buffers as calling xfs_buf_offset() is not free and
it occurs a *lot* in those cases.

Fixes: 929f8b0deb83 ("xfs: optimise xfs_buf_item_size/format for contiguous regions")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 47549cfa61cd821c8925c8d2b5dda2e7763605d1..0ee6fa9efd18c9abbd9608053b8c93c5fbd2b77a 100644 (file)
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -57,6 +57,10 @@ xfs_buf_log_format_size(
                        (blfp->blf_map_size * sizeof(blfp->blf_data_map[0]));
 }
 
+/*
+ * We only have to worry about discontiguous buffer range straddling on unmapped
+ * buffers. Everything else will have a contiguous data region we can copy from.
+ */
 static inline bool
 xfs_buf_item_straddle(
        struct xfs_buf          *bp,
@@ -66,6 +70,9 @@ xfs_buf_item_straddle(
 {
        void                    *first, *last;
 
+       if (bp->b_page_count == 1 || !(bp->b_flags & XBF_UNMAPPED))
+               return false;
+
        first = xfs_buf_offset(bp, offset + (first_bit << XFS_BLF_SHIFT));
        last = xfs_buf_offset(bp,
                        offset + ((first_bit + nbits) << XFS_BLF_SHIFT));
@@ -133,11 +140,13 @@ xfs_buf_item_size_segment(
        return;
 
 slow_scan:
-       /* Count the first bit we jumped out of the above loop from */
-       (*nvecs)++;
-       *nbytes += XFS_BLF_CHUNK;
+       ASSERT(bp->b_addr == NULL);
        last_bit = first_bit;
+       nbits = 1;
        while (last_bit != -1) {
+
+               *nbytes += XFS_BLF_CHUNK;
+
                /*
                 * This takes the bit number to start looking from and
                 * returns the next set bit from there.  It returns -1
@@ -152,6 +161,8 @@ slow_scan:
                 * else keep scanning the current set of bits.
                 */
                if (next_bit == -1) {
+                       if (first_bit != last_bit)
+                               (*nvecs)++;
                        break;
                } else if (next_bit != last_bit + 1 ||
                           xfs_buf_item_straddle(bp, offset, first_bit, nbits)) {
@@ -163,7 +174,6 @@ slow_scan:
                        last_bit++;
                        nbits++;
                }
-               *nbytes += XFS_BLF_CHUNK;
        }
 }