On Wed, 2022-05-18 at 11:55 -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> When we're validating a recovered xattr log item during log recovery,
> we
> should check the name before starting to allocate resources. This
> isn't
> strictly necessary on its own, but it means that we won't bother with
> huge memory allocations during recovery if the attr name is garbage,
> which will simplify the changes in the next patch.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Ok, this looks good to me
Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/xfs_attr_item.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index fd0a74f3ef45..4976b1ddc09f 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -688,16 +688,23 @@ xlog_recover_attri_commit_pass2(
> struct xfs_mount *mp = log->l_mp;
> struct xfs_attri_log_item *attrip;
> struct xfs_attri_log_format *attri_formatp;
> + const void *attr_name;
> int region = 0;
>
> attri_formatp = item->ri_buf[region].i_addr;
> + attr_name = item->ri_buf[1].i_addr;
>
> - /* Validate xfs_attri_log_format */
> + /* Validate xfs_attri_log_format before the large memory
> allocation */
> if (!xfs_attri_validate(mp, attri_formatp)) {
> XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> return -EFSCORRUPTED;
> }
>
> + if (!xfs_attr_namecheck(attr_name, attri_formatp-
> >alfi_name_len)) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> + return -EFSCORRUPTED;
> + }
> +
> /* memory alloc failure will cause replay to abort */
> attrip = xfs_attri_init(mp, attri_formatp->alfi_name_len,
> attri_formatp->alfi_value_len);
> @@ -713,12 +720,6 @@ xlog_recover_attri_commit_pass2(
> memcpy(attrip->attri_name, item->ri_buf[region].i_addr,
> attrip->attri_name_len);
>
> - if (!xfs_attr_namecheck(attrip->attri_name, attrip-
> >attri_name_len)) {
> - XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> - error = -EFSCORRUPTED;
> - goto out;
> - }
> -
> if (attrip->attri_value_len > 0) {
> region++;
> memcpy(attrip->attri_value, item-
> >ri_buf[region].i_addr,
>
