On 11/13/2016 12:23 PM, Andreas Gruenbacher wrote: > Al, > > here is a fix for a regression introduced in commit 6c6ef9f2 (which is > contained in v4.9-rc1). The fix was originally posted on November 3 [*], with > no reaction: > > https://patchwork.kernel.org/patch/9410885/ > https://patchwork.kernel.org/patch/9411123/ > > So may I again ask you to have a look, preferably before we slip yet another -rc? This is a critical regression fix for Smack. Without the fix it is impossible to properly configure certain system services. > > Thanks, > Andreas > > -- > > The IOP_XATTR flag is set on sockfs because sockfs supports getting the > "system.sockprotoname" xattr. Since commit 6c6ef9f2, this flag is checked for > setxattr support as well. This is wrong on sockfs because security xattr > support there is supposed to be provided by security_inode_setsecurity. The > smack security module relies on socket labels (xattrs). > > 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 after > that as well. > > Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx> > --- > fs/xattr.c | 22 ++++++++++++++-------- > net/socket.c | 15 +++++++++++++++ > 2 files changed, 29 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..9820725 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -341,8 +341,23 @@ 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, > + &sockfs_security_xattr_handler, > NULL > }; > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
