On Tue, Dec 19, 2017 at 04:23:29PM +1100, Dave Chinner wrote:
> On Wed, Dec 13, 2017 at 03:58:55PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Create a function to perform structure verification for short form
> > extended attributes.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_attr_leaf.c | 70 +++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/libxfs/xfs_attr_leaf.h | 1 +
> > 2 files changed, 71 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index 1274872..a5033f0 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -873,6 +873,76 @@ xfs_attr_shortform_allfit(
> > return xfs_attr_shortform_bytesfit(dp, bytes);
> > }
> >
> > +/* Verify the consistency of an inline attribute fork. */
> > +xfs_failaddr_t
> > +xfs_attr_shortform_verify(
> > + struct xfs_inode *ip)
> > +{
> > + struct xfs_attr_shortform *sfp;
> > + struct xfs_attr_sf_entry *sfep;
> > + struct xfs_attr_sf_entry *next_sfep;
> > + char *endp;
> > + struct xfs_ifork *ifp;
> > + int i;
> > + int size;
> > +
> > + ASSERT(ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL);
> > + ifp = XFS_IFORK_PTR(ip, XFS_ATTR_FORK);
> > + sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
> > + size = ifp->if_bytes;
> > +
> > + /*
> > + * Give up if the attribute is way too short.
> > + */
> > + if (size < sizeof(struct xfs_attr_sf_hdr))
> > + return __this_address;
> > +
> > + endp = (char *)sfp + size;
> > +
> > + /* Check all reported entries */
> > + sfep = &sfp->list[0];
> > + for (i = 0; i < sfp->hdr.count; i++) {
> > + /*
> > + * struct xfs_attr_sf_entry has a variable length.
> > + * Check the fixed-offset parts of the structure are
> > + * within the data buffer.
> > + */
> > + if (((char *)sfep + sizeof(*sfep)) >= endp)
> > + return __this_address;
> > +
> > + /* Don't allow names with known bad length. */
> > + if (sfep->namelen == 0)
> > + return __this_address;
> > +
> > + /*
> > + * Check that the variable-length part of the structure is
> > + * within the data buffer. The next entry starts after the
> > + * name component, so nextentry is an acceptable test.
> > + */
> > + next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
> > + if ((char *)next_sfep > endp)
> > + return __this_address;
> > +
> > + /*
> > + * Check for unknown flags. Short form doesn't support
> > + * the incomplete or local bits.
> > + */
> > + if (sfep->flags & ~(XFS_ATTR_ROOT | XFS_ATTR_SECURE))
> > + return __this_address;
> > +
> > + /* Check for invalid namespace combinations. */
> > + if ((sfep->flags & XFS_ATTR_ROOT) &&
> > + (sfep->flags & XFS_ATTR_SECURE))
> > + return __this_address;
>
> #define ATTR_SF_FLAGS (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
XFS_ATTR_NSP_ONDISK_MASK?
>
> if (sfep->flags & ~ATTR_SF_FLAGS)
> return __this_address;
> if ((sfep->flags & ATTR_SF_FLAGS) == ATTR_SF_FLAGS)
> return __this_address;
Also, can we just use hweight8() > 1 here? We only allow one namespace
bit per attr.
--D
>
> Otherwise looks ok.
>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
