On Mon, Oct 28, 2019 at 09:03:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Add missing structure checks in the attribute leaf verifier.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> fs/xfs/libxfs/xfs_attr_leaf.c | 67 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index f0089e862216..bb80a01bcca9 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -232,6 +232,61 @@ xfs_attr3_leaf_hdr_to_disk(
> }
> }
>
> +static xfs_failaddr_t
> +xfs_attr3_leaf_verify_entry(
> + struct xfs_mount *mp,
> + char *buf_end,
> + struct xfs_attr_leafblock *leaf,
> + struct xfs_attr3_icleaf_hdr *leafhdr,
> + struct xfs_attr_leaf_entry *ent,
> + int idx,
> + __u32 *last_hashval)
> +{
> + struct xfs_attr_leaf_name_local *lentry;
> + struct xfs_attr_leaf_name_remote *rentry;
> + char *name_end;
> + unsigned int nameidx;
> + unsigned int namesize;
> + __u32 hashval;
> +
> + /* hash order check */
> + hashval = be32_to_cpu(ent->hashval);
> + if (hashval < *last_hashval)
> + return __this_address;
> + *last_hashval = hashval;
> +
> + nameidx = be16_to_cpu(ent->nameidx);
> + if (nameidx < leafhdr->firstused || nameidx >= mp->m_attr_geo->blksize)
> + return __this_address;
> +
> + /*
> + * Check the name information. The namelen fields are u8 so we can't
> + * possibly exceed the maximum name length of 255 bytes.
> + */
> + if (ent->flags & XFS_ATTR_LOCAL) {
> + lentry = xfs_attr3_leaf_name_local(leaf, idx);
> + namesize = xfs_attr_leaf_entsize_local(lentry->namelen,
> + be16_to_cpu(lentry->valuelen));
> + name_end = (char *)lentry + namesize;
> + if (lentry->namelen == 0)
> + return __this_address;
> + } else {
> + rentry = xfs_attr3_leaf_name_remote(leaf, idx);
> + namesize = xfs_attr_leaf_entsize_remote(rentry->namelen);
> + name_end = (char *)rentry + namesize;
> + if (rentry->namelen == 0)
> + return __this_address;
> + if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
> + rentry->valueblk == 0)
> + return __this_address;
> + }
> +
> + if (name_end > buf_end)
> + return __this_address;
> +
> + return NULL;
> +}
> +
> static xfs_failaddr_t
> xfs_attr3_leaf_verify(
> struct xfs_buf *bp)
> @@ -240,7 +295,10 @@ xfs_attr3_leaf_verify(
> struct xfs_mount *mp = bp->b_mount;
> struct xfs_attr_leafblock *leaf = bp->b_addr;
> struct xfs_attr_leaf_entry *entries;
> + struct xfs_attr_leaf_entry *ent;
> + char *buf_end;
> uint32_t end; /* must be 32bit - see below */
> + __u32 last_hashval = 0;
> int i;
> xfs_failaddr_t fa;
>
> @@ -273,8 +331,13 @@ xfs_attr3_leaf_verify(
> (char *)bp->b_addr + ichdr.firstused)
> return __this_address;
>
> - /* XXX: need to range check rest of attr header values */
> - /* XXX: hash order check? */
> + buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
> + for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
> + fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
> + ent, i, &last_hashval);
> + if (fa)
> + return fa;
> + }
>
> /*
> * Quickly check the freemap information. Attribute data has to be
>
