| Message ID | 20191212105433.1692-4-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/33] xfs: clear kernel only flags in XFS_IOC_ATTRMULTI_BY_HANDLE | expand |
On Thu, Dec 12, 2019 at 11:54:03AM +0100, Christoph Hellwig wrote: > We should not just invalidate the ACL when setting the underlying > attribute, but also when removing it. The ioctl interface gets that > right, but the normal xattr inteface skipped the xfs_forget_acl due > to an early return. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Shouldn't someone have a testcase for this? --D > --- > fs/xfs/xfs_xattr.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > index 383f0203d103..2288f20ae282 100644 > --- a/fs/xfs/xfs_xattr.c > +++ b/fs/xfs/xfs_xattr.c > @@ -74,10 +74,11 @@ xfs_xattr_set(const struct xattr_handler *handler, struct dentry *unused, > if (flags & XATTR_REPLACE) > xflags |= ATTR_REPLACE; > > - if (!value) > - return xfs_attr_remove(ip, (unsigned char *)name, xflags); > - error = xfs_attr_set(ip, (unsigned char *)name, > + if (value) > + error = xfs_attr_set(ip, (unsigned char *)name, > (void *)value, size, xflags); > + else > + error = xfs_attr_remove(ip, (unsigned char *)name, xflags); > if (!error) > xfs_forget_acl(inode, name, xflags); > > -- > 2.20.1 >
On Wed, Dec 18, 2019 at 01:31:08PM -0800, Darrick J. Wong wrote: > On Thu, Dec 12, 2019 at 11:54:03AM +0100, Christoph Hellwig wrote: > > We should not just invalidate the ACL when setting the underlying > > attribute, but also when removing it. The ioctl interface gets that > > right, but the normal xattr inteface skipped the xfs_forget_acl due > > to an early return. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Shouldn't someone have a testcase for this? adding Andreas, who wrote this code originally.
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index 383f0203d103..2288f20ae282 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c @@ -74,10 +74,11 @@ xfs_xattr_set(const struct xattr_handler *handler, struct dentry *unused, if (flags & XATTR_REPLACE) xflags |= ATTR_REPLACE; - if (!value) - return xfs_attr_remove(ip, (unsigned char *)name, xflags); - error = xfs_attr_set(ip, (unsigned char *)name, + if (value) + error = xfs_attr_set(ip, (unsigned char *)name, (void *)value, size, xflags); + else + error = xfs_attr_remove(ip, (unsigned char *)name, xflags); if (!error) xfs_forget_acl(inode, name, xflags);
We should not just invalidate the ACL when setting the underlying attribute, but also when removing it. The ioctl interface gets that right, but the normal xattr inteface skipped the xfs_forget_acl due to an early return. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_xattr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)