On 11/3/2016 6:45 AM, Andreas Gruenbacher wrote: > Casey, the first patch broke filesystems that support setxattr for some xattrs > but not security xattrs. Here's an updated patch; could you please test?
This patch does not fix the problem. I am back to EOPTNOTSUP. > > Al, does this look mergeable? > > Thanks, > Andreas > > -- > > The IOP_XATTR flag is set on sockfs because sockfs supports getting the > "system.sockprotoname" xattr. Commit 6c6ef9f2 started to check this > flag for setxattr support as well. This is wrong on sockfs because > security xattr support there is provided by security_inode_setsecurity. > > Fix this by adding a security xattr handler on sockfs that returns > -EAGAIN and by checking for -EAGAIN in setxattr. > > We cannot simply check for -EOPNOTSUPP in setxattr because there are > filesystems that neither have direct security xattr support nor support > via security_inode_setsecurity. A more proper fix might be to move the > call to security_inode_setsecurity into sockfs, but it's not clear to me > if that is safe: we would end up calling security_inode_post_setxattr as > well. > > Signed-off-by: Andreas Gruenbacher <[email protected]> > --- > fs/xattr.c | 22 ++++++++++++++-------- > net/socket.c | 14 ++++++++++++++ > 2 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/fs/xattr.c b/fs/xattr.c > index 3368659..2d13b4e 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -170,7 +170,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const > char *name, > const void *value, size_t size, int flags) > { > struct inode *inode = dentry->d_inode; > - int error = -EOPNOTSUPP; > + int error = -EAGAIN; > int issec = !strncmp(name, XATTR_SECURITY_PREFIX, > XATTR_SECURITY_PREFIX_LEN); > > @@ -183,15 +183,21 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const > char *name, > security_inode_post_setxattr(dentry, name, value, > size, flags); > } > - } else if (issec) { > - const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; > - > + } else { > if (unlikely(is_bad_inode(inode))) > return -EIO; > - error = security_inode_setsecurity(inode, suffix, value, > - size, flags); > - if (!error) > - fsnotify_xattr(dentry); > + } > + if (error == -EAGAIN) { > + error = -EOPNOTSUPP; > + > + if (issec) { > + const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; > + > + error = security_inode_setsecurity(inode, suffix, value, > + size, flags); > + if (!error) > + fsnotify_xattr(dentry); > + } > } > > return error; > diff --git a/net/socket.c b/net/socket.c > index 5a9bf5e..816392a 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -341,6 +341,20 @@ static const struct xattr_handler sockfs_xattr_handler = > { > .get = sockfs_xattr_get, > }; > > +static int sockfs_security_xattr_set(const struct xattr_handler *handler, > + struct dentry *dentry, struct inode *inode, > + const char *suffix, const void *value, > + size_t size, int flags) > +{ > + /* Handled by LSM. */ > + return -EAGAIN; > +} > + > +static const struct xattr_handler sockfs_security_xattr_handler = { > + .prefix = XATTR_SECURITY_PREFIX, > + .set = sockfs_security_xattr_set, > +}; > + > static const struct xattr_handler *sockfs_xattr_handlers[] = { > &sockfs_xattr_handler, > NULL
