On Tue, Nov 15, 2022 at 12:49:49AM -0800, Christoph Hellwig wrote:
> On Tue, Nov 15, 2022 at 12:30:42PM +1100, Dave Chinner wrote:
> > +/*
> > + * Check that the iomap passed to us is still valid for the given offset and
> > + * length.
> > + */
> > +static bool
> > +xfs_iomap_valid(
> > + struct inode *inode,
> > + const struct iomap *iomap)
> > +{
> > + struct xfs_inode *ip = XFS_I(inode);
> > + u64 cookie = 0;
> > +
> > + if (iomap->flags & IOMAP_F_XATTR) {
> > + cookie = READ_ONCE(ip->i_af.if_seq);
> > + } else {
> > + if ((iomap->flags & IOMAP_F_SHARED) && ip->i_cowfp)
> > + cookie = (u64)READ_ONCE(ip->i_cowfp->if_seq) << 32;
> > + cookie |= READ_ONCE(ip->i_df.if_seq);
> > + }
> > + return cookie == iomap->validity_cookie;
>
> How can this be called with IOMAP_F_XATTR set?
It can't be. *Yet*.
However: ->iomap_begin can be called to map an attribute fork, and
->iomap_valid *could* be called on the attr iomap that is returned.
At which point, the iomap needs to be self-describing and the
validation cookie needs to hold the attr fork sequence number for
everything to work.
Failing to handle/validate attr iomaps correctly would clearly be an
XFS implementation bug and making it work correctly is trivial and
costs nothing. Hence making it work is a no-brainer - leaving it as
a landmine for someone else to trip over in future is a poor outcome
for everyone.
> Also the code seems to duplicate xfs_iomap_inode_sequence, so
> we just call that:
>
> return cookie == xfs_iomap_inode_sequence(XFS_I(inode), iomap->flags);
Yup, will fix.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
