xfs: fix memcpy fortify errors in BUI log format copying
authorDarrick J. Wong <djwong@kernel.org>
Thu, 20 Oct 2022 23:17:41 +0000 (16:17 -0700)
committerDarrick J. Wong <djwong@kernel.org>
Mon, 31 Oct 2022 15:58:19 +0000 (08:58 -0700)
Starting in 6.1, CONFIG_FORTIFY_SOURCE checks the length parameter of
memcpy.  Unfortunately, it doesn't handle flex arrays correctly:

------------[ cut here ]------------
memcpy: detected field-spanning write (size 48) of single field "dst_bui_fmt" at fs/xfs/xfs_bmap_item.c:628 (size 16)

Fix this by refactoring the xfs_bui_copy_format function to handle the
copying of the head and the flex array members separately.  While we're
at it, fix a minor validation deficiency in the recovery function.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
fs/xfs/xfs_bmap_item.c
fs/xfs/xfs_ondisk.h

index 51f66e9..a1da620 100644 (file)
@@ -608,28 +608,18 @@ static const struct xfs_item_ops xfs_bui_item_ops = {
        .iop_relog      = xfs_bui_item_relog,
 };
 
-/*
- * Copy an BUI format buffer from the given buf, and into the destination
- * BUI format structure.  The BUI/BUD items were designed not to need any
- * special alignment handling.
- */
-static int
+static inline void
 xfs_bui_copy_format(
-       struct xfs_log_iovec            *buf,
-       struct xfs_bui_log_format       *dst_bui_fmt)
+       struct xfs_bui_log_format       *dst,
+       const struct xfs_bui_log_format *src)
 {
-       struct xfs_bui_log_format       *src_bui_fmt;
-       uint                            len;
+       unsigned int                    i;
 
-       src_bui_fmt = buf->i_addr;
-       len = xfs_bui_log_format_sizeof(src_bui_fmt->bui_nextents);
+       memcpy(dst, src, offsetof(struct xfs_bui_log_format, bui_extents));
 
-       if (buf->i_len == len) {
-               memcpy(dst_bui_fmt, src_bui_fmt, len);
-               return 0;
-       }
-       XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
-       return -EFSCORRUPTED;
+       for (i = 0; i < src->bui_nextents; i++)
+               memcpy(&dst->bui_extents[i], &src->bui_extents[i],
+                               sizeof(struct xfs_map_extent));
 }
 
 /*
@@ -646,23 +636,31 @@ xlog_recover_bui_commit_pass2(
        struct xlog_recover_item        *item,
        xfs_lsn_t                       lsn)
 {
-       int                             error;
        struct xfs_mount                *mp = log->l_mp;
        struct xfs_bui_log_item         *buip;
        struct xfs_bui_log_format       *bui_formatp;
+       size_t                          len;
 
        bui_formatp = item->ri_buf[0].i_addr;
 
+       if (item->ri_buf[0].i_len < xfs_bui_log_format_sizeof(0)) {
+               XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+               return -EFSCORRUPTED;
+       }
+
        if (bui_formatp->bui_nextents != XFS_BUI_MAX_FAST_EXTENTS) {
                XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
                return -EFSCORRUPTED;
        }
-       buip = xfs_bui_init(mp);
-       error = xfs_bui_copy_format(&item->ri_buf[0], &buip->bui_format);
-       if (error) {
-               xfs_bui_item_free(buip);
-               return error;
+
+       len = xfs_bui_log_format_sizeof(bui_formatp->bui_nextents);
+       if (item->ri_buf[0].i_len != len) {
+               XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log->l_mp);
+               return -EFSCORRUPTED;
        }
+
+       buip = xfs_bui_init(mp);
+       xfs_bui_copy_format(&buip->bui_format, bui_formatp);
        atomic_set(&buip->bui_next_extent, bui_formatp->bui_nextents);
        /*
         * Insert the intent into the AIL directly and drop one reference so
index 758702b..56917e2 100644 (file)
@@ -134,6 +134,11 @@ xfs_check_ondisk_structs(void)
        XFS_CHECK_STRUCT_SIZE(struct xfs_trans_header,          16);
        XFS_CHECK_STRUCT_SIZE(struct xfs_attri_log_format,      40);
        XFS_CHECK_STRUCT_SIZE(struct xfs_attrd_log_format,      16);
+       XFS_CHECK_STRUCT_SIZE(struct xfs_bui_log_format,        16);
+       XFS_CHECK_STRUCT_SIZE(struct xfs_bud_log_format,        16);
+       XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent,            32);
+
+       XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents,        16);
 
        /*
         * The v5 superblock format extended several v4 header structures with