On Mon, 2022-10-24 at 14:32 -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
Ok, makes sense:
Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/xfs_bmap_item.c | 46 ++++++++++++++++++++++----------------
> --------
> fs/xfs/xfs_ondisk.h | 5 +++++
> 2 files changed, 27 insertions(+), 24 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 51f66e982484..a1da6205252b 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -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;
> }
> +
> + 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);
> - error = xfs_bui_copy_format(&item->ri_buf[0], &buip-
> >bui_format);
> - if (error) {
> - xfs_bui_item_free(buip);
> - return error;
> - }
> + 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
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 758702b9495f..56917e236370 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -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
>
