| Message ID | 157232182873.593721.10436299832453896943.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: more metadata verifier tightening | expand |
On Mon, Oct 28, 2019 at 09:03:48PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Add missing structure checks in the attribute leaf verifier. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > 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 >
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