All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: Casey Schaufler <casey@schaufler-ca.com>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: Andreas Gruenbacher <agruenba@redhat.com>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Stephen Smalley <sds@tycho.nsa.gov>
Subject: [PATCH] xattr: Fix setting security xattrs on sockfs
Date: Thu,  3 Nov 2016 14:45:06 +0100	[thread overview]
Message-ID: <1478180706-9456-1-git-send-email-agruenba@redhat.com> (raw)
In-Reply-To: 57e9db81-d785-98ab-dd52-47c1f82c76c3@schaufler-ca.com

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?

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 <agruenba@redhat.com>
---
 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
-- 
2.7.4

  reply	other threads:[~2016-11-03 13:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31 21:55 Problem with setxattr on sockfs with Smack after 971df15bd54ad46e907046ff33750a137b2f0096 Casey Schaufler
2016-11-01 16:10 ` Casey Schaufler
2016-11-02 19:34   ` [PATCH] " Andreas Gruenbacher
2016-11-02 20:09     ` Casey Schaufler
2016-11-03 13:45       ` Andreas Gruenbacher [this message]
2016-11-03 15:51         ` [PATCH] xattr: Fix setting security xattrs on sockfs Casey Schaufler
2016-11-03 16:00           ` Andreas Gruenbacher
2016-11-03 16:25             ` Casey Schaufler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1478180706-9456-1-git-send-email-agruenba@redhat.com \
    --to=agruenba@redhat.com \
    --cc=casey@schaufler-ca.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sds@tycho.nsa.gov \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.