From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758496AbcKCPv1 (ORCPT ); Thu, 3 Nov 2016 11:51:27 -0400 Received: from nm7-vm3.bullet.mail.ne1.yahoo.com ([98.138.91.137]:41707 "EHLO nm7-vm3.bullet.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758478AbcKCPvZ (ORCPT ); Thu, 3 Nov 2016 11:51:25 -0400 X-Yahoo-Newman-Id: 341582.80511.bm@smtp113.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: R6jitccVM1kvmVVkfHFK7UIxXmw_liLKTGJtSfJa.AIEi4M j8LuNwuJxgbpAkT3Uiq6.jJ0y.UXx_HirDqB3AJcEcqu970g9MaRCleq.q30 ItzCuSWLvOieuhLNf.ZbA4UiVOA40QJsNNJ5REKrDMf1JyWAsWJLyphjW3vs 4kfHaD5akUuvjvLknlNXOwwzuTBxz0S56UJLJJtD_nyh.rqTVXYimdcPeN.5 2Xv99fhV_uVlhdQ2R0T8fA9l8t_.XnkEgjr9CSernjs8ylBHR2uhFPSR2BRP spKx5Z6TuWuCAYF.6CuqAR9D75Q6dnY3aTNV6sY.a39kUPeIbnKQTp2CLtgS daJ56FJfn6abGETI9n27zQpU6fo_yzmOPqAp4pACI1.Nbq6ypp1vG3bqBgbn WisVHXqCeEqi_lSJOAolZQ4bJsrV8kjyjYSGYfJkPih0RUkFtTWHj9CIbIIG t8RCyA0kpcaatSeLc.nXQVcpI.YLoDpXvaiC5TCq37t7v8jOiknMda_tTkS4 gW_eNNwv_2hfklbtYrPojtptXFsn9bd1.P77fOKTqVzJLv_sM X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [PATCH] xattr: Fix setting security xattrs on sockfs To: Andreas Gruenbacher , Al Viro References: <1478115261-29669-1-git-send-email-agruenba@redhat.com> <57e9db81-d785-98ab-dd52-47c1f82c76c3@schaufler-ca.com> <1478180706-9456-1-git-send-email-agruenba@redhat.com> Cc: linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, Stephen Smalley From: Casey Schaufler Message-ID: Date: Thu, 3 Nov 2016 08:51:20 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1478180706-9456-1-git-send-email-agruenba@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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