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. Since we're already fixing problems with BUI item copying,
> we
> should fix it everything else.
>
> Refactor the xfs_rui_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>
Looks good to me
Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/xfs_ondisk.h | 3 ++
> fs/xfs/xfs_rmap_item.c | 58 ++++++++++++++++++++++----------------
> ----------
> 2 files changed, 30 insertions(+), 31 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index e20d2844b0c5..19c1df00b48e 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -138,11 +138,14 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(struct xfs_bud_log_format, 16);
> XFS_CHECK_STRUCT_SIZE(struct xfs_cui_log_format, 16);
> XFS_CHECK_STRUCT_SIZE(struct xfs_cud_log_format, 16);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_rui_log_format, 16);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format, 16);
> XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent, 32);
> XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent, 16);
>
> XFS_CHECK_OFFSET(struct xfs_bui_log_format,
> bui_extents, 16);
> XFS_CHECK_OFFSET(struct xfs_cui_log_format,
> cui_extents, 16);
> + XFS_CHECK_OFFSET(struct xfs_rui_log_format,
> rui_extents, 16);
>
> /*
> * The v5 superblock format extended several v4 header
> structures with
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index fef92e02f3bb..27047e73f582 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -155,31 +155,6 @@ xfs_rui_init(
> return ruip;
> }
>
> -/*
> - * Copy an RUI format buffer from the given buf, and into the
> destination
> - * RUI format structure. The RUI/RUD items were designed not to
> need any
> - * special alignment handling.
> - */
> -STATIC int
> -xfs_rui_copy_format(
> - struct xfs_log_iovec *buf,
> - struct xfs_rui_log_format *dst_rui_fmt)
> -{
> - struct xfs_rui_log_format *src_rui_fmt;
> - uint len;
> -
> - src_rui_fmt = buf->i_addr;
> - len = xfs_rui_log_format_sizeof(src_rui_fmt->rui_nextents);
> -
> - if (buf->i_len != len) {
> - XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
> - return -EFSCORRUPTED;
> - }
> -
> - memcpy(dst_rui_fmt, src_rui_fmt, len);
> - return 0;
> -}
> -
> static inline struct xfs_rud_log_item *RUD_ITEM(struct xfs_log_item
> *lip)
> {
> return container_of(lip, struct xfs_rud_log_item, rud_item);
> @@ -652,6 +627,20 @@ static const struct xfs_item_ops
> xfs_rui_item_ops = {
> .iop_relog = xfs_rui_item_relog,
> };
>
> +static inline void
> +xfs_rui_copy_format(
> + struct xfs_rui_log_format *dst,
> + const struct xfs_rui_log_format *src)
> +{
> + unsigned int i;
> +
> + memcpy(dst, src, offsetof(struct xfs_rui_log_format,
> rui_extents));
> +
> + for (i = 0; i < src->rui_nextents; i++)
> + memcpy(&dst->rui_extents[i], &src->rui_extents[i],
> + sizeof(struct xfs_map_extent));
> +}
> +
> /*
> * This routine is called to create an in-core extent rmap update
> * item from the rui format structure which was logged on disk.
> @@ -666,19 +655,26 @@ xlog_recover_rui_commit_pass2(
> struct xlog_recover_item *item,
> xfs_lsn_t lsn)
> {
> - int error;
> struct xfs_mount *mp = log->l_mp;
> struct xfs_rui_log_item *ruip;
> struct xfs_rui_log_format *rui_formatp;
> + size_t len;
>
> rui_formatp = item->ri_buf[0].i_addr;
>
> + if (item->ri_buf[0].i_len < xfs_rui_log_format_sizeof(0)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log-
> >l_mp);
> + return -EFSCORRUPTED;
> + }
> +
> + len = xfs_rui_log_format_sizeof(rui_formatp->rui_nextents);
> + if (item->ri_buf[0].i_len != len) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, log-
> >l_mp);
> + return -EFSCORRUPTED;
> + }
> +
> ruip = xfs_rui_init(mp, rui_formatp->rui_nextents);
> - error = xfs_rui_copy_format(&item->ri_buf[0], &ruip-
> >rui_format);
> - if (error) {
> - xfs_rui_item_free(ruip);
> - return error;
> - }
> + xfs_rui_copy_format(&ruip->rui_format, rui_formatp);
> atomic_set(&ruip->rui_next_extent, rui_formatp-
> >rui_nextents);
> /*
> * Insert the intent into the AIL directly and drop one
> reference so
>
