[PATCH] Security/sysfs: Enable security xattrs to be set on sysfs files, directories, and symlinks

[PATCH] Security/sysfs: Enable security xattrs to be set on sysfs files, directories, and symlinks

[David P. Quigley]

This is revision three of the sysfs labeling patch set. Changes from version
two are that the new LSM hooks proposed in the last patch have been removed and
replaced with several hooks that were developed for labeled NFS. Instead of
storing the secid which Casey objected to it was replaced with the result of a
call to the new security_inode_getsecctx call. This call was developed to
handle the case where we may have a multiple xattr lsm. So in the new case
sysfs will make a call to getsecctx when the new xattr is set and this will
retreive all of the security information in one string. Subsequent inode
instantiations will take this value out of the sysfs dentry and use the new
security_inode_notifysecctx to place it into the sysfs inode. The third hook
setsecctx is there to round off the set and performs both the setting of incore
state and on disk value of the xattrs. This isn't used for sysfs because there
is no disk backing store for the inode.

 fs/sysfs/dir.c             |    1 +
 fs/sysfs/inode.c           |  135 ++++++++++++++++++++++++++++++++------------
 fs/sysfs/symlink.c         |    2 +
 fs/sysfs/sysfs.h           |   12 ++++-
 fs/xattr.c                 |   55 ++++++++++++++----
 include/linux/security.h   |   55 ++++++++++++++++++
 include/linux/xattr.h      |    1 +
 security/capability.c      |   17 ++++++
 security/security.c        |   18 ++++++
 security/selinux/hooks.c   |   33 +++++++++++
 security/smack/smack_lsm.c |   24 ++++++++
 11 files changed, 303 insertions(+), 50 deletions(-)
 

[PATCH 1/3] VFS: Factor out part of vfs_setxattr so it can be called from the SELinux hook for inode_setsecctx.

[David P. Quigley]

This factors out the part of the vfs_setxattr function that performs the
setting of the xattr and its notification. This is needed so the SELinux
implementation of inode_setsecctx can handle the setting of the xattr while
maintaining the proper separation of layers.

Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
 fs/xattr.c            |   55 +++++++++++++++++++++++++++++++++++++-----------
 include/linux/xattr.h |    1 +
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 1c3d0af..6d4f6d3 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -66,22 +66,28 @@ xattr_permission(struct inode *inode, const char *name, int mask)
 	return inode_permission(inode, mask);
 }
 
-int
-vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
-		size_t size, int flags)
+/**
+ *  __vfs_setxattr_noperm - perform setxattr operation without performing
+ *  permission checks.
+ *
+ *  @dentry - object to perform setxattr on
+ *  @name - xattr name to set
+ *  @value - value to set @name to
+ *  @size - size of @value
+ *  @flags - flags to pass into filesystem operations
+ *
+ *  returns the result of the internal setxattr or setsecurity operations.
+ *
+ *  This function requires the caller to lock the inode's i_mutex before it
+ *  is executed. It also assumes that the caller will make the appropriate
+ *  permission checks.
+ */
+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;
-
-	error = xattr_permission(inode, name, MAY_WRITE);
-	if (error)
-		return error;
+	int error = -EOPNOTSUPP;
 
-	mutex_lock(&inode->i_mutex);
-	error = security_inode_setxattr(dentry, name, value, size, flags);
-	if (error)
-		goto out;
-	error = -EOPNOTSUPP;
 	if (inode->i_op->setxattr) {
 		error = inode->i_op->setxattr(dentry, name, value, size, flags);
 		if (!error) {
@@ -97,6 +103,29 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 		if (!error)
 			fsnotify_xattr(dentry);
 	}
+
+	return error;
+}
+
+
+int
+vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags)
+{
+	struct inode *inode = dentry->d_inode;
+	int error;
+
+	error = xattr_permission(inode, name, MAY_WRITE);
+	if (error)
+		return error;
+
+	mutex_lock(&inode->i_mutex);
+	error = security_inode_setxattr(dentry, name, value, size, flags);
+	if (error)
+		goto out;
+
+	error = __vfs_setxattr_noperm(dentry, name, value, size, flags);
+
 out:
 	mutex_unlock(&inode->i_mutex);
 	return error;
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d131e35..5c84af8 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -49,6 +49,7 @@ struct xattr_handler {
 ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
 ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
+int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int);
 int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
 int vfs_removexattr(struct dentry *, const char *);
 
-- 
1.5.6.6

Re: [PATCH 1/3] VFS: Factor out part of vfs_setxattr so it can be called from the SELinux hook for inode_setsecctx.

[Serge E. Hallyn]

Quoting David P. Quigley (dpquigl@tycho.nsa.gov):
> This factors out the part of the vfs_setxattr function that performs the
> setting of the xattr and its notification. This is needed so the SELinux
> implementation of inode_setsecctx can handle the setting of the xattr while
> maintaining the proper separation of layers.
> 
> Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>

Acked-by: Serge Hallyn <serue@us.ibm.com>

> ---
>  fs/xattr.c            |   55 +++++++++++++++++++++++++++++++++++++-----------
>  include/linux/xattr.h |    1 +
>  2 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 1c3d0af..6d4f6d3 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -66,22 +66,28 @@ xattr_permission(struct inode *inode, const char *name, int mask)
>  	return inode_permission(inode, mask);
>  }
> 
> -int
> -vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
> -		size_t size, int flags)
> +/**
> + *  __vfs_setxattr_noperm - perform setxattr operation without performing
> + *  permission checks.
> + *
> + *  @dentry - object to perform setxattr on
> + *  @name - xattr name to set
> + *  @value - value to set @name to
> + *  @size - size of @value
> + *  @flags - flags to pass into filesystem operations
> + *
> + *  returns the result of the internal setxattr or setsecurity operations.
> + *
> + *  This function requires the caller to lock the inode's i_mutex before it
> + *  is executed. It also assumes that the caller will make the appropriate
> + *  permission checks.
> + */
> +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;
> -
> -	error = xattr_permission(inode, name, MAY_WRITE);
> -	if (error)
> -		return error;
> +	int error = -EOPNOTSUPP;
> 
> -	mutex_lock(&inode->i_mutex);
> -	error = security_inode_setxattr(dentry, name, value, size, flags);
> -	if (error)
> -		goto out;
> -	error = -EOPNOTSUPP;
>  	if (inode->i_op->setxattr) {
>  		error = inode->i_op->setxattr(dentry, name, value, size, flags);
>  		if (!error) {
> @@ -97,6 +103,29 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
>  		if (!error)
>  			fsnotify_xattr(dentry);
>  	}
> +
> +	return error;
> +}
> +
> +
> +int
> +vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
> +		size_t size, int flags)
> +{
> +	struct inode *inode = dentry->d_inode;
> +	int error;
> +
> +	error = xattr_permission(inode, name, MAY_WRITE);
> +	if (error)
> +		return error;
> +
> +	mutex_lock(&inode->i_mutex);
> +	error = security_inode_setxattr(dentry, name, value, size, flags);
> +	if (error)
> +		goto out;
> +
> +	error = __vfs_setxattr_noperm(dentry, name, value, size, flags);
> +
>  out:
>  	mutex_unlock(&inode->i_mutex);
>  	return error;
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index d131e35..5c84af8 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -49,6 +49,7 @@ struct xattr_handler {
>  ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
>  ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
>  ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
> +int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int);
>  int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
>  int vfs_removexattr(struct dentry *, const char *);
> 
> -- 
> 1.5.6.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] LSM/SELinux: inode_{get,set,notify}secctx hooks to access LSM security context information.

[David P. Quigley]

This patch introduces three new hooks. The inode_getsecctx hook is used to get
all relevant information from an LSM about an inode. The inode_setsecctx is
used to set both the in-core and on-disk state for the inode based on a context
derived from inode_getsecctx.The final hook inode_notifysecctx will notify the
LSM of a change for the in-core state of the inode in question. These hooks are
for use in the labeled NFS code and addresses concerns of how to set security
on an inode in a multi-xattr LSM. For historical reasons Stephen Smalley's
explanation of the reason for these hooks is pasted below.

Quote Stephen Smalley

inode_setsecctx:  Change the security context of an inode.  Updates the
in core security context managed by the security module and invokes the
fs code as needed (via __vfs_setxattr_noperm) to update any backing
xattrs that represent the context.  Example usage:  NFS server invokes
this hook to change the security context in its incore inode and on the
backing file system to a value provided by the client on a SETATTR
operation.

inode_notifysecctx:  Notify the security module of what the security
context of an inode should be.  Initializes the incore security context
managed by the security module for this inode.  Example usage:  NFS
client invokes this hook to initialize the security context in its
incore inode to the value provided by the server for the file when the
server returned the file's attributes to the client.

Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
 include/linux/security.h   |   55 ++++++++++++++++++++++++++++++++++++++++++++
 security/capability.c      |   17 +++++++++++++
 security/security.c        |   18 ++++++++++++++
 security/selinux/hooks.c   |   29 +++++++++++++++++++++++
 security/smack/smack_lsm.c |   24 +++++++++++++++++++
 5 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 1f16eea..a88fc49 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1351,6 +1351,41 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	audit_rule_init.
  *	@rule contains the allocated rule
  *
+ * @inode_notifysecctx:
+ *	Notify the security module of what the security context of an inode
+ *	should be.  Initializes the incore security context managed by the
+ *	security module for this inode.  Example usage:  NFS client invokes
+ *	this hook to initialize the security context in its incore inode to the
+ *	value provided by the server for the file when the server returned the
+ *	file's attributes to the client.
+ *
+ * 	Must be called with inode->i_mutex locked.
+ *
+ * 	@inode we wish to set the security context of.
+ * 	@ctx contains the string which we wish to set in the inode.
+ * 	@ctxlen contains the length of @ctx.
+ *
+ * @inode_setsecctx:
+ * 	Change the security context of an inode.  Updates the
+ * 	incore security context managed by the security module and invokes the
+ * 	fs code as needed (via __vfs_setxattr_noperm) to update any backing
+ * 	xattrs that represent the context.  Example usage:  NFS server invokes
+ * 	this hook to change the security context in its incore inode and on the
+ * 	backing filesystem to a value provided by the client on a SETATTR
+ * 	operation.
+ *
+ * 	Must be called with inode->i_mutex locked.
+ *
+ * 	@dentry contains the inode we wish to set the security context of.
+ * 	@ctx contains the string which we wish to set in the inode.
+ * 	@ctxlen contains the length of @ctx.
+ *
+ * @inode_getsecctx:
+ * 	Returns a string containing all relavent security context information
+ *
+ * 	@inode we wish to set the security context of.
+ *	@ctx is a pointer in which to place the allocated security context.
+ *	@ctxlen points to the place to put the length of @ctx.
  * This is the main security structure.
  */
 struct security_operations {
@@ -1556,6 +1591,10 @@ struct security_operations {
 	int (*secctx_to_secid) (const char *secdata, u32 seclen, u32 *secid);
 	void (*release_secctx) (char *secdata, u32 seclen);
 
+	int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
+	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
+	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
+
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect) (struct socket *sock,
 				    struct socket *other, struct sock *newsk);
@@ -1796,6 +1835,9 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
 int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
 void security_release_secctx(char *secdata, u32 seclen);
 
+int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
+int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
+int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };
@@ -2537,6 +2579,19 @@ static inline int security_secctx_to_secid(const char *secdata,
 static inline void security_release_secctx(char *secdata, u32 seclen)
 {
 }
+
+static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+{
+	return -EOPNOTSUPP;
+}
+static inline int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+{
+	return -EOPNOTSUPP;
+}
+static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/capability.c b/security/capability.c
index 88f752e..f77684f 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -792,6 +792,20 @@ static void cap_release_secctx(char *secdata, u32 seclen)
 {
 }
 
+static int cap_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+{
+	return 0;
+}
+
+static int cap_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+{
+	return 0;
+}
+
+static int cap_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
+{
+	return 0;
+}
 #ifdef CONFIG_KEYS
 static int cap_key_alloc(struct key *key, const struct cred *cred,
 			 unsigned long flags)
@@ -992,6 +1006,9 @@ void security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, secid_to_secctx);
 	set_to_cap_if_null(ops, secctx_to_secid);
 	set_to_cap_if_null(ops, release_secctx);
+	set_to_cap_if_null(ops, inode_notifysecctx);
+	set_to_cap_if_null(ops, inode_setsecctx);
+	set_to_cap_if_null(ops, inode_getsecctx);
 #ifdef CONFIG_SECURITY_NETWORK
 	set_to_cap_if_null(ops, unix_stream_connect);
 	set_to_cap_if_null(ops, unix_may_send);
diff --git a/security/security.c b/security/security.c
index dc7674f..42a6d28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -959,6 +959,24 @@ void security_release_secctx(char *secdata, u32 seclen)
 }
 EXPORT_SYMBOL(security_release_secctx);
 
+int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+{
+	return security_ops->inode_notifysecctx(inode, ctx, ctxlen);
+}
+EXPORT_SYMBOL(security_inode_notifysecctx);
+
+int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+{
+	return security_ops->inode_setsecctx(dentry, ctx, ctxlen);
+}
+EXPORT_SYMBOL(security_inode_setsecctx);
+
+int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
+{
+	return security_ops->inode_getsecctx(inode, ctx, ctxlen);
+}
+EXPORT_SYMBOL(security_inode_getsecctx);
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 int security_unix_stream_connect(struct socket *sock, struct socket *other,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8d8b69c..f1d5677 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5252,6 +5252,32 @@ static void selinux_release_secctx(char *secdata, u32 seclen)
 	kfree(secdata);
 }
 
+/*
+ *	called with inode->i_mutex locked
+ */
+static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+{
+	return selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, ctxlen, 0);
+}
+
+/*
+ *	called with inode->i_mutex locked
+ */
+static int selinux_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+{
+	return __vfs_setxattr_noperm(dentry, XATTR_NAME_SELINUX, ctx, ctxlen, 0);
+}
+
+static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
+{
+	int len = 0;
+	len = selinux_inode_getsecurity(inode, XATTR_SELINUX_SUFFIX,
+						ctx, true);
+	if (len < 0)
+		return len;
+	*ctxlen = len;
+	return 0;
+}
 #ifdef CONFIG_KEYS
 
 static int selinux_key_alloc(struct key *k, const struct cred *cred,
@@ -5448,6 +5474,9 @@ static struct security_operations selinux_ops = {
 	.secid_to_secctx =		selinux_secid_to_secctx,
 	.secctx_to_secid =		selinux_secctx_to_secid,
 	.release_secctx =		selinux_release_secctx,
+	.inode_notifysecctx =		selinux_inode_notifysecctx,
+	.inode_setsecctx =		selinux_inode_setsecctx,
+	.inode_getsecctx =		selinux_inode_getsecctx,
 
 	.unix_stream_connect =		selinux_socket_unix_stream_connect,
 	.unix_may_send =		selinux_socket_unix_may_send,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0023182..62af40e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3029,6 +3029,27 @@ static void smack_release_secctx(char *secdata, u32 seclen)
 {
 }
 
+static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+{
+	return smack_inode_setsecurity(inode, XATTR_SMACK_SUFFIX, ctx, ctxlen, 0);
+}
+
+static int smack_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+{
+	return __vfs_setxattr_noperm(dentry, XATTR_NAME_SMACK, ctx, ctxlen, 0);	
+}
+
+static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
+{
+	int len = 0;
+	len = smack_inode_getsecurity(inode, XATTR_SMACK_SUFFIX, ctx, true);
+
+	if (len < 0)
+		return len;
+	*ctxlen = len;
+	return 0;
+}
+
 struct security_operations smack_ops = {
 	.name =				"smack",
 
@@ -3155,6 +3176,9 @@ struct security_operations smack_ops = {
 	.secid_to_secctx = 		smack_secid_to_secctx,
 	.secctx_to_secid = 		smack_secctx_to_secid,
 	.release_secctx = 		smack_release_secctx,
+	.inode_notifysecctx =		smack_inode_notifysecctx,
+	.inode_setsecctx =		smack_inode_setsecctx,
+	.inode_getsecctx =		smack_inode_getsecctx,
 };
 
 
-- 
1.5.6.6

Re: [PATCH 2/3] LSM/SELinux: inode_{get,set,notify}secctx hooks to access LSM security context information.

[Serge E. Hallyn]

Quoting David P. Quigley (dpquigl@tycho.nsa.gov):
> This patch introduces three new hooks. The inode_getsecctx hook is used to get
> all relevant information from an LSM about an inode. The inode_setsecctx is

The 'security_xyz_getctx' namespace is getting a bit polluted.  I suspect
I should take this as a hint to rename my
	selinux_file_get_ctx()
to
	selinux_checkpoint_file()
or
	selinux_checkpoint_file_ctx()

to more clearly distinguish it from your hooks and the existing
secid_to_secctx() set of .hooks

> used to set both the in-core and on-disk state for the inode based on a context
> derived from inode_getsecctx.The final hook inode_notifysecctx will notify the
> LSM of a change for the in-core state of the inode in question. These hooks are
> for use in the labeled NFS code and addresses concerns of how to set security
> on an inode in a multi-xattr LSM. For historical reasons Stephen Smalley's
> explanation of the reason for these hooks is pasted below.
> 
> Quote Stephen Smalley
> 
> inode_setsecctx:  Change the security context of an inode.  Updates the
> in core security context managed by the security module and invokes the
> fs code as needed (via __vfs_setxattr_noperm) to update any backing
> xattrs that represent the context.  Example usage:  NFS server invokes
> this hook to change the security context in its incore inode and on the
> backing file system to a value provided by the client on a SETATTR
> operation.

So this is only to be called by kernel code, right?  Hence no
authorization checks needed?

> inode_notifysecctx:  Notify the security module of what the security
> context of an inode should be.  Initializes the incore security context
> managed by the security module for this inode.  Example usage:  NFS
> client invokes this hook to initialize the security context in its
> incore inode to the value provided by the server for the file when the
> server returned the file's attributes to the client.
> 
> Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>

Acked-by: Serge Hallyn <serue@us.ibm.com>

> ---
>  include/linux/security.h   |   55 ++++++++++++++++++++++++++++++++++++++++++++
>  security/capability.c      |   17 +++++++++++++
>  security/security.c        |   18 ++++++++++++++
>  security/selinux/hooks.c   |   29 +++++++++++++++++++++++
>  security/smack/smack_lsm.c |   24 +++++++++++++++++++
>  5 files changed, 143 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1f16eea..a88fc49 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1351,6 +1351,41 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
>   *	audit_rule_init.
>   *	@rule contains the allocated rule
>   *
> + * @inode_notifysecctx:
> + *	Notify the security module of what the security context of an inode
> + *	should be.  Initializes the incore security context managed by the
> + *	security module for this inode.  Example usage:  NFS client invokes
> + *	this hook to initialize the security context in its incore inode to the
> + *	value provided by the server for the file when the server returned the
> + *	file's attributes to the client.
> + *
> + * 	Must be called with inode->i_mutex locked.
> + *
> + * 	@inode we wish to set the security context of.
> + * 	@ctx contains the string which we wish to set in the inode.
> + * 	@ctxlen contains the length of @ctx.
> + *
> + * @inode_setsecctx:
> + * 	Change the security context of an inode.  Updates the
> + * 	incore security context managed by the security module and invokes the
> + * 	fs code as needed (via __vfs_setxattr_noperm) to update any backing
> + * 	xattrs that represent the context.  Example usage:  NFS server invokes
> + * 	this hook to change the security context in its incore inode and on the
> + * 	backing filesystem to a value provided by the client on a SETATTR
> + * 	operation.

If someone is using fs/cachefiles, do these changes get automatically
kicked up to the cached view?

> + * 	Must be called with inode->i_mutex locked.
> + *
> + * 	@dentry contains the inode we wish to set the security context of.
> + * 	@ctx contains the string which we wish to set in the inode.
> + * 	@ctxlen contains the length of @ctx.
> + *
> + * @inode_getsecctx:
> + * 	Returns a string containing all relavent security context information

Is it worth spelling out even more clearly that the string needs to be freed
by the caller?

> + *
> + * 	@inode we wish to set the security context of.
> + *	@ctx is a pointer in which to place the allocated security context.
> + *	@ctxlen points to the place to put the length of @ctx.
>   * This is the main security structure.
>   */
>  struct security_operations {
> @@ -1556,6 +1591,10 @@ struct security_operations {
>  	int (*secctx_to_secid) (const char *secdata, u32 seclen, u32 *secid);
>  	void (*release_secctx) (char *secdata, u32 seclen);
> 
> +	int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
> +	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
> +	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  	int (*unix_stream_connect) (struct socket *sock,
>  				    struct socket *other, struct sock *newsk);
> @@ -1796,6 +1835,9 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
>  int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
>  void security_release_secctx(char *secdata, u32 seclen);
> 
> +int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> +int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> +int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
>  #else /* CONFIG_SECURITY */
>  struct security_mnt_opts {
>  };
> @@ -2537,6 +2579,19 @@ static inline int security_secctx_to_secid(const char *secdata,
>  static inline void security_release_secctx(char *secdata, u32 seclen)
>  {
>  }
> +
> +static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif	/* CONFIG_SECURITY */
> 
>  #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/capability.c b/security/capability.c
> index 88f752e..f77684f 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -792,6 +792,20 @@ static void cap_release_secctx(char *secdata, u32 seclen)
>  {
>  }
> 
> +static int cap_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> +{
> +	return 0;
> +}
> +
> +static int cap_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> +{
> +	return 0;
> +}
> +
> +static int cap_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +{
> +	return 0;
> +}
>  #ifdef CONFIG_KEYS
>  static int cap_key_alloc(struct key *key, const struct cred *cred,
>  			 unsigned long flags)
> @@ -992,6 +1006,9 @@ void security_fixup_ops(struct security_operations *ops)
>  	set_to_cap_if_null(ops, secid_to_secctx);
>  	set_to_cap_if_null(ops, secctx_to_secid);
>  	set_to_cap_if_null(ops, release_secctx);
> +	set_to_cap_if_null(ops, inode_notifysecctx);
> +	set_to_cap_if_null(ops, inode_setsecctx);
> +	set_to_cap_if_null(ops, inode_getsecctx);
>  #ifdef CONFIG_SECURITY_NETWORK
>  	set_to_cap_if_null(ops, unix_stream_connect);
>  	set_to_cap_if_null(ops, unix_may_send);
> diff --git a/security/security.c b/security/security.c
> index dc7674f..42a6d28 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -959,6 +959,24 @@ void security_release_secctx(char *secdata, u32 seclen)
>  }
>  EXPORT_SYMBOL(security_release_secctx);
> 
> +int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> +{
> +	return security_ops->inode_notifysecctx(inode, ctx, ctxlen);
> +}
> +EXPORT_SYMBOL(security_inode_notifysecctx);
> +
> +int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> +{
> +	return security_ops->inode_setsecctx(dentry, ctx, ctxlen);
> +}
> +EXPORT_SYMBOL(security_inode_setsecctx);
> +
> +int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +{
> +	return security_ops->inode_getsecctx(inode, ctx, ctxlen);
> +}
> +EXPORT_SYMBOL(security_inode_getsecctx);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
> 
>  int security_unix_stream_connect(struct socket *sock, struct socket *other,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 8d8b69c..f1d5677 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5252,6 +5252,32 @@ static void selinux_release_secctx(char *secdata, u32 seclen)
>  	kfree(secdata);
>  }
> 
> +/*
> + *	called with inode->i_mutex locked
> + */
> +static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> +{
> +	return selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, ctxlen, 0);
> +}
> +
> +/*
> + *	called with inode->i_mutex locked
> + */
> +static int selinux_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> +{
> +	return __vfs_setxattr_noperm(dentry, XATTR_NAME_SELINUX, ctx, ctxlen, 0);
> +}
> +
> +static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +{
> +	int len = 0;
> +	len = selinux_inode_getsecurity(inode, XATTR_SELINUX_SUFFIX,
> +						ctx, true);
> +	if (len < 0)
> +		return len;
> +	*ctxlen = len;
> +	return 0;
> +}
>  #ifdef CONFIG_KEYS
> 
>  static int selinux_key_alloc(struct key *k, const struct cred *cred,
> @@ -5448,6 +5474,9 @@ static struct security_operations selinux_ops = {
>  	.secid_to_secctx =		selinux_secid_to_secctx,
>  	.secctx_to_secid =		selinux_secctx_to_secid,
>  	.release_secctx =		selinux_release_secctx,
> +	.inode_notifysecctx =		selinux_inode_notifysecctx,
> +	.inode_setsecctx =		selinux_inode_setsecctx,
> +	.inode_getsecctx =		selinux_inode_getsecctx,
> 
>  	.unix_stream_connect =		selinux_socket_unix_stream_connect,
>  	.unix_may_send =		selinux_socket_unix_may_send,
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 0023182..62af40e 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3029,6 +3029,27 @@ static void smack_release_secctx(char *secdata, u32 seclen)
>  {
>  }
> 
> +static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> +{
> +	return smack_inode_setsecurity(inode, XATTR_SMACK_SUFFIX, ctx, ctxlen, 0);
> +}
> +
> +static int smack_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> +{
> +	return __vfs_setxattr_noperm(dentry, XATTR_NAME_SMACK, ctx, ctxlen, 0);	
> +}
> +
> +static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +{
> +	int len = 0;
> +	len = smack_inode_getsecurity(inode, XATTR_SMACK_SUFFIX, ctx, true);
> +
> +	if (len < 0)
> +		return len;
> +	*ctxlen = len;
> +	return 0;
> +}
> +
>  struct security_operations smack_ops = {
>  	.name =				"smack",
> 
> @@ -3155,6 +3176,9 @@ struct security_operations smack_ops = {
>  	.secid_to_secctx = 		smack_secid_to_secctx,
>  	.secctx_to_secid = 		smack_secctx_to_secid,
>  	.release_secctx = 		smack_release_secctx,
> +	.inode_notifysecctx =		smack_inode_notifysecctx,
> +	.inode_setsecctx =		smack_inode_setsecctx,
> +	.inode_getsecctx =		smack_inode_getsecctx,
>  };
> 
> 
> -- 
> 1.5.6.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] LSM/SELinux: inode_{get,set,notify}secctx hooks to access LSM security context information.

[Stephen Smalley]

On Fri, 2009-09-04 at 10:49 -0500, Serge E. Hallyn wrote:
> Quoting David P. Quigley (dpquigl@tycho.nsa.gov):
> > This patch introduces three new hooks. The inode_getsecctx hook is used to get
> > all relevant information from an LSM about an inode. The inode_setsecctx is
> 
> The 'security_xyz_getctx' namespace is getting a bit polluted.  I suspect
> I should take this as a hint to rename my
> 	selinux_file_get_ctx()
> to
> 	selinux_checkpoint_file()

Yes, I think that would be clearer.

> or
> 	selinux_checkpoint_file_ctx()
> 
> to more clearly distinguish it from your hooks and the existing
> secid_to_secctx() set of .hooks
> 
> > used to set both the in-core and on-disk state for the inode based on a context
> > derived from inode_getsecctx.The final hook inode_notifysecctx will notify the
> > LSM of a change for the in-core state of the inode in question. These hooks are
> > for use in the labeled NFS code and addresses concerns of how to set security
> > on an inode in a multi-xattr LSM. For historical reasons Stephen Smalley's
> > explanation of the reason for these hooks is pasted below.
> > 
> > Quote Stephen Smalley
> > 
> > inode_setsecctx:  Change the security context of an inode.  Updates the
> > in core security context managed by the security module and invokes the
> > fs code as needed (via __vfs_setxattr_noperm) to update any backing
> > xattrs that represent the context.  Example usage:  NFS server invokes
> > this hook to change the security context in its incore inode and on the
> > backing file system to a value provided by the client on a SETATTR
> > operation.
> 
> So this is only to be called by kernel code, right?  Hence no
> authorization checks needed?

Correct.  And just to clarify, these hooks have been previously
discussed for the labeled NFS work, but were on hold until a user of
them (e.g. the labeled NFS support itself) was ready to upstream.  But
they turn out to be useful for sysfs labeling as well since they address
Casey's concerns about not passing secids and supporting multi-xattr
LSMs, so the sysfs patch was reworked to use them.

> 
> > inode_notifysecctx:  Notify the security module of what the security
> > context of an inode should be.  Initializes the incore security context
> > managed by the security module for this inode.  Example usage:  NFS
> > client invokes this hook to initialize the security context in its
> > incore inode to the value provided by the server for the file when the
> > server returned the file's attributes to the client.
> > 
> > Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
> 
> Acked-by: Serge Hallyn <serue@us.ibm.com>

-- 
Stephen Smalley
National Security Agency

[PATCH 3/3] sysfs: Add labeling support for sysfs

[David P. Quigley]

This patch adds a setxattr handler to the file, directory, and symlink
inode_operations structures for sysfs. The patch uses hooks introduced in the
previous patch to handle the getting and setting of security information for
the sysfs inodes. As was suggested by Eric Biederman the struct iattr in the
sysfs_dirent structure has been replaced by a structure which contains the
iattr, secdata and secdata length to allow the changes to persist in the event
that the inode representing the sysfs_dirent is evicted. Because sysfs only
stores this information when a change is made all the optional data is moved
into one dynamically allocated field.

This patch addresses an issue where SELinux was denying virtd access to the PCI
configuration entries in sysfs. The lack of setxattr handlers for sysfs
required that a single label be assigned to all entries in sysfs. Granting virtd
access to every entry in sysfs is not an acceptable solution so fine grained
labeling of sysfs is required such that individual entries can be labeled
appropriately.

Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
 fs/sysfs/dir.c           |    1 +
 fs/sysfs/inode.c         |  135 +++++++++++++++++++++++++++++++++------------
 fs/sysfs/symlink.c       |    2 +
 fs/sysfs/sysfs.h         |   12 ++++-
 security/selinux/hooks.c |    4 ++
 5 files changed, 117 insertions(+), 37 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 14f2d71..0050fc4 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -760,6 +760,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 const struct inode_operations sysfs_dir_inode_operations = {
 	.lookup		= sysfs_lookup,
 	.setattr	= sysfs_setattr,
+	.setxattr	= sysfs_setxattr,
 };
 
 static void remove_dir(struct sysfs_dirent *sd)
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 555f0ff..9889bf2 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -18,6 +18,8 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
+#include <linux/xattr.h>
+#include <linux/security.h>
 #include "sysfs.h"
 
 extern struct super_block * sysfs_sb;
@@ -35,6 +37,7 @@ static struct backing_dev_info sysfs_backing_dev_info = {
 
 static const struct inode_operations sysfs_inode_operations ={
 	.setattr	= sysfs_setattr,
+	.setxattr	= sysfs_setxattr,
 };
 
 int __init sysfs_inode_init(void)
@@ -42,18 +45,37 @@ int __init sysfs_inode_init(void)
 	return bdi_init(&sysfs_backing_dev_info);
 }
 
+struct sysfs_inode_attrs * sysfs_init_inode_attrs(struct sysfs_dirent * sd)
+{
+	struct sysfs_inode_attrs * attrs;
+	struct iattr * iattrs;
+
+	attrs = kzalloc(sizeof(struct sysfs_inode_attrs), GFP_KERNEL);
+	if(!attrs)
+		return NULL;
+	iattrs = &attrs->ia_iattr;
+
+	/* assign default attributes */
+	iattrs->ia_mode = sd->s_mode;
+	iattrs->ia_uid = 0;
+	iattrs->ia_gid = 0;
+	iattrs->ia_atime = iattrs->ia_mtime = iattrs->ia_ctime = CURRENT_TIME;
+
+	return attrs;
+}
 int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
 {
 	struct inode * inode = dentry->d_inode;
 	struct sysfs_dirent * sd = dentry->d_fsdata;
-	struct iattr * sd_iattr;
+	struct sysfs_inode_attrs * sd_attrs;
+	struct iattr * iattrs;
 	unsigned int ia_valid = iattr->ia_valid;
 	int error;
 
 	if (!sd)
 		return -EINVAL;
 
-	sd_iattr = sd->s_iattr;
+	sd_attrs = sd->s_iattr;
 
 	error = inode_change_ok(inode, iattr);
 	if (error)
@@ -65,42 +87,78 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
 	if (error)
 		return error;
 
-	if (!sd_iattr) {
+	if (!sd_attrs) {
 		/* setting attributes for the first time, allocate now */
-		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
-		if (!sd_iattr)
+		sd_attrs = sysfs_init_inode_attrs(sd);
+		if (!sd_attrs)
 			return -ENOMEM;
-		/* assign default attributes */
-		sd_iattr->ia_mode = sd->s_mode;
-		sd_iattr->ia_uid = 0;
-		sd_iattr->ia_gid = 0;
-		sd_iattr->ia_atime = sd_iattr->ia_mtime = sd_iattr->ia_ctime = CURRENT_TIME;
-		sd->s_iattr = sd_iattr;
+		sd->s_iattr = sd_attrs;
+	} else {
+		/* attributes were changed atleast once in past */
+		iattrs = &sd_attrs->ia_iattr;
+
+		if (ia_valid & ATTR_UID)
+			iattrs->ia_uid = iattr->ia_uid;
+		if (ia_valid & ATTR_GID)
+			iattrs->ia_gid = iattr->ia_gid;
+		if (ia_valid & ATTR_ATIME)
+			iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_MTIME)
+			iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_CTIME)
+			iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_MODE) {
+			umode_t mode = iattr->ia_mode;
+
+			if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
+				mode &= ~S_ISGID;
+			iattrs->ia_mode = sd->s_mode = mode;
+		}
 	}
+	return error;
+}
 
-	/* attributes were changed atleast once in past */
-
-	if (ia_valid & ATTR_UID)
-		sd_iattr->ia_uid = iattr->ia_uid;
-	if (ia_valid & ATTR_GID)
-		sd_iattr->ia_gid = iattr->ia_gid;
-	if (ia_valid & ATTR_ATIME)
-		sd_iattr->ia_atime = timespec_trunc(iattr->ia_atime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		sd_iattr->ia_mtime = timespec_trunc(iattr->ia_mtime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		sd_iattr->ia_ctime = timespec_trunc(iattr->ia_ctime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MODE) {
-		umode_t mode = iattr->ia_mode;
-
-		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
-			mode &= ~S_ISGID;
-		sd_iattr->ia_mode = sd->s_mode = mode;
-	}
+int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags)
+{
+	struct sysfs_dirent *sd = dentry->d_fsdata;
+	struct sysfs_inode_attrs *iattrs;
+	char *suffix;
+	char *secdata;
+	int error;
+	u32 secdata_len = 0;
+
+	if (!sd)
+		return -EINVAL;
+	if (!sd->s_iattr)
+		sd->s_iattr = sysfs_init_inode_attrs(sd);
+	if (!sd->s_iattr)
+		return -ENOMEM;
+
+	iattrs = sd->s_iattr;
+
+	if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
+		const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
+		error = security_inode_setsecurity(dentry->d_inode, suffix,
+						value, size, flags);
+		if (error)
+			goto out;
+		error = security_inode_getsecctx(dentry->d_inode,
+						&secdata, &secdata_len);
+		if (error)
+			goto out;
+		if(iattrs->ia_secdata)
+			security_release_secctx(iattrs->ia_secdata,
+						iattrs->ia_secdata_len);
+		iattrs->ia_secdata = secdata;
+		iattrs->ia_secdata_len = secdata_len;
 
+	} else
+		return -EINVAL;
+out:
 	return error;
 }
 
@@ -146,6 +204,7 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd)
 static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
 {
 	struct bin_attribute *bin_attr;
+	struct sysfs_inode_attrs * iattrs;
 
 	inode->i_private = sysfs_get(sd);
 	inode->i_mapping->a_ops = &sysfs_aops;
@@ -154,16 +213,20 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
 	inode->i_ino = sd->s_ino;
 	lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
 
-	if (sd->s_iattr) {
+	iattrs = sd->s_iattr;
+	if (iattrs) {
 		/* sysfs_dirent has non-default attributes
 		 * get them for the new inode from persistent copy
 		 * in sysfs_dirent
 		 */
-		set_inode_attr(inode, sd->s_iattr);
+		set_inode_attr(inode, &iattrs->ia_iattr);
+		if (iattrs->ia_secdata)
+			security_inode_notifysecctx(inode,
+						iattrs->ia_secdata,
+						iattrs->ia_secdata_len);
 	} else
 		set_default_inode_attr(inode, sd->s_mode);
 
-
 	/* initialize inode according to type */
 	switch (sysfs_type(sd)) {
 	case SYSFS_DIR:
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 1d897ad..c5081ad 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -16,6 +16,7 @@
 #include <linux/kobject.h>
 #include <linux/namei.h>
 #include <linux/mutex.h>
+#include <linux/security.h>
 
 #include "sysfs.h"
 
@@ -209,6 +210,7 @@ static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd, void *co
 }
 
 const struct inode_operations sysfs_symlink_inode_operations = {
+	.setxattr = sysfs_setxattr,
 	.readlink = generic_readlink,
 	.follow_link = sysfs_follow_link,
 	.put_link = sysfs_put_link,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 3fa0d98..af4c4e7 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -8,6 +8,8 @@
  * This file is released under the GPLv2.
  */
 
+#include <linux/fs.h>
+
 struct sysfs_open_dirent;
 
 /* type-specific structures for sysfs_dirent->s_* union members */
@@ -31,6 +33,12 @@ struct sysfs_elem_bin_attr {
 	struct hlist_head	buffers;
 };
 
+struct sysfs_inode_attrs {
+	struct iattr	ia_iattr;
+	void		*ia_secdata;
+	u32		ia_secdata_len;
+};
+
 /*
  * sysfs_dirent - the building block of sysfs hierarchy.  Each and
  * every sysfs node is represented by single sysfs_dirent.
@@ -56,7 +64,7 @@ struct sysfs_dirent {
 	unsigned int		s_flags;
 	ino_t			s_ino;
 	umode_t			s_mode;
-	struct iattr		*s_iattr;
+	struct sysfs_inode_attrs *s_iattr;
 };
 
 #define SD_DEACTIVATED_BIAS		INT_MIN
@@ -148,6 +156,8 @@ static inline void __sysfs_put(struct sysfs_dirent *sd)
 struct inode *sysfs_get_inode(struct sysfs_dirent *sd);
 void sysfs_delete_inode(struct inode *inode);
 int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
+int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags);
 int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name);
 int sysfs_inode_init(void);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f1d5677..38ed894 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -448,6 +448,10 @@ static int sb_finish_set_opts(struct super_block *sb)
 	    sbsec->behavior > ARRAY_SIZE(labeling_behaviors))
 		sbsec->flags &= ~SE_SBLABELSUPP;
 
+	/* Special handling for sysfs. Is genfs but also has setxattr handler*/
+	if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0)
+		sbsec->flags |= SE_SBLABELSUPP;
+
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
 
-- 
1.5.6.6

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Serge E. Hallyn]

Quoting David P. Quigley (dpquigl@tycho.nsa.gov):
> This patch adds a setxattr handler to the file, directory, and symlink
> inode_operations structures for sysfs. The patch uses hooks introduced in the
> previous patch to handle the getting and setting of security information for
> the sysfs inodes. As was suggested by Eric Biederman the struct iattr in the
> sysfs_dirent structure has been replaced by a structure which contains the
> iattr, secdata and secdata length to allow the changes to persist in the event
> that the inode representing the sysfs_dirent is evicted. Because sysfs only
> stores this information when a change is made all the optional data is moved
> into one dynamically allocated field.
> 
> This patch addresses an issue where SELinux was denying virtd access to the PCI
> configuration entries in sysfs. The lack of setxattr handlers for sysfs
> required that a single label be assigned to all entries in sysfs. Granting virtd
> access to every entry in sysfs is not an acceptable solution so fine grained
> labeling of sysfs is required such that individual entries can be labeled
> appropriately.
> 
> Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>

Hmm, all looks good to me...

Acked-by: Serge Hallyn <serue@us.ibm.com>

> ---
>  fs/sysfs/dir.c           |    1 +
>  fs/sysfs/inode.c         |  135 +++++++++++++++++++++++++++++++++------------
>  fs/sysfs/symlink.c       |    2 +
>  fs/sysfs/sysfs.h         |   12 ++++-
>  security/selinux/hooks.c |    4 ++
>  5 files changed, 117 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
> index 14f2d71..0050fc4 100644
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -760,6 +760,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
>  const struct inode_operations sysfs_dir_inode_operations = {
>  	.lookup		= sysfs_lookup,
>  	.setattr	= sysfs_setattr,
> +	.setxattr	= sysfs_setxattr,
>  };
> 
>  static void remove_dir(struct sysfs_dirent *sd)
> diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
> index 555f0ff..9889bf2 100644
> --- a/fs/sysfs/inode.c
> +++ b/fs/sysfs/inode.c
> @@ -18,6 +18,8 @@
>  #include <linux/capability.h>
>  #include <linux/errno.h>
>  #include <linux/sched.h>
> +#include <linux/xattr.h>
> +#include <linux/security.h>
>  #include "sysfs.h"
> 
>  extern struct super_block * sysfs_sb;
> @@ -35,6 +37,7 @@ static struct backing_dev_info sysfs_backing_dev_info = {
> 
>  static const struct inode_operations sysfs_inode_operations ={
>  	.setattr	= sysfs_setattr,
> +	.setxattr	= sysfs_setxattr,
>  };
> 
>  int __init sysfs_inode_init(void)
> @@ -42,18 +45,37 @@ int __init sysfs_inode_init(void)
>  	return bdi_init(&sysfs_backing_dev_info);
>  }
> 
> +struct sysfs_inode_attrs * sysfs_init_inode_attrs(struct sysfs_dirent * sd)
> +{
> +	struct sysfs_inode_attrs * attrs;
> +	struct iattr * iattrs;
> +
> +	attrs = kzalloc(sizeof(struct sysfs_inode_attrs), GFP_KERNEL);
> +	if(!attrs)
> +		return NULL;
> +	iattrs = &attrs->ia_iattr;
> +
> +	/* assign default attributes */
> +	iattrs->ia_mode = sd->s_mode;
> +	iattrs->ia_uid = 0;
> +	iattrs->ia_gid = 0;
> +	iattrs->ia_atime = iattrs->ia_mtime = iattrs->ia_ctime = CURRENT_TIME;
> +
> +	return attrs;
> +}
>  int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
>  {
>  	struct inode * inode = dentry->d_inode;
>  	struct sysfs_dirent * sd = dentry->d_fsdata;
> -	struct iattr * sd_iattr;
> +	struct sysfs_inode_attrs * sd_attrs;
> +	struct iattr * iattrs;
>  	unsigned int ia_valid = iattr->ia_valid;
>  	int error;
> 
>  	if (!sd)
>  		return -EINVAL;
> 
> -	sd_iattr = sd->s_iattr;
> +	sd_attrs = sd->s_iattr;
> 
>  	error = inode_change_ok(inode, iattr);
>  	if (error)
> @@ -65,42 +87,78 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
>  	if (error)
>  		return error;
> 
> -	if (!sd_iattr) {
> +	if (!sd_attrs) {
>  		/* setting attributes for the first time, allocate now */
> -		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
> -		if (!sd_iattr)
> +		sd_attrs = sysfs_init_inode_attrs(sd);
> +		if (!sd_attrs)
>  			return -ENOMEM;
> -		/* assign default attributes */
> -		sd_iattr->ia_mode = sd->s_mode;
> -		sd_iattr->ia_uid = 0;
> -		sd_iattr->ia_gid = 0;
> -		sd_iattr->ia_atime = sd_iattr->ia_mtime = sd_iattr->ia_ctime = CURRENT_TIME;
> -		sd->s_iattr = sd_iattr;
> +		sd->s_iattr = sd_attrs;
> +	} else {
> +		/* attributes were changed atleast once in past */
> +		iattrs = &sd_attrs->ia_iattr;
> +
> +		if (ia_valid & ATTR_UID)
> +			iattrs->ia_uid = iattr->ia_uid;
> +		if (ia_valid & ATTR_GID)
> +			iattrs->ia_gid = iattr->ia_gid;
> +		if (ia_valid & ATTR_ATIME)
> +			iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
> +					inode->i_sb->s_time_gran);
> +		if (ia_valid & ATTR_MTIME)
> +			iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
> +					inode->i_sb->s_time_gran);
> +		if (ia_valid & ATTR_CTIME)
> +			iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
> +					inode->i_sb->s_time_gran);
> +		if (ia_valid & ATTR_MODE) {
> +			umode_t mode = iattr->ia_mode;
> +
> +			if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
> +				mode &= ~S_ISGID;
> +			iattrs->ia_mode = sd->s_mode = mode;
> +		}
>  	}
> +	return error;
> +}
> 
> -	/* attributes were changed atleast once in past */
> -
> -	if (ia_valid & ATTR_UID)
> -		sd_iattr->ia_uid = iattr->ia_uid;
> -	if (ia_valid & ATTR_GID)
> -		sd_iattr->ia_gid = iattr->ia_gid;
> -	if (ia_valid & ATTR_ATIME)
> -		sd_iattr->ia_atime = timespec_trunc(iattr->ia_atime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_MTIME)
> -		sd_iattr->ia_mtime = timespec_trunc(iattr->ia_mtime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_CTIME)
> -		sd_iattr->ia_ctime = timespec_trunc(iattr->ia_ctime,
> -						inode->i_sb->s_time_gran);
> -	if (ia_valid & ATTR_MODE) {
> -		umode_t mode = iattr->ia_mode;
> -
> -		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
> -			mode &= ~S_ISGID;
> -		sd_iattr->ia_mode = sd->s_mode = mode;
> -	}
> +int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
> +		size_t size, int flags)
> +{
> +	struct sysfs_dirent *sd = dentry->d_fsdata;
> +	struct sysfs_inode_attrs *iattrs;
> +	char *suffix;
> +	char *secdata;
> +	int error;
> +	u32 secdata_len = 0;
> +
> +	if (!sd)
> +		return -EINVAL;
> +	if (!sd->s_iattr)
> +		sd->s_iattr = sysfs_init_inode_attrs(sd);
> +	if (!sd->s_iattr)
> +		return -ENOMEM;
> +
> +	iattrs = sd->s_iattr;
> +
> +	if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
> +		const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
> +		error = security_inode_setsecurity(dentry->d_inode, suffix,
> +						value, size, flags);
> +		if (error)
> +			goto out;
> +		error = security_inode_getsecctx(dentry->d_inode,
> +						&secdata, &secdata_len);
> +		if (error)
> +			goto out;
> +		if(iattrs->ia_secdata)
> +			security_release_secctx(iattrs->ia_secdata,
> +						iattrs->ia_secdata_len);
> +		iattrs->ia_secdata = secdata;
> +		iattrs->ia_secdata_len = secdata_len;
> 
> +	} else
> +		return -EINVAL;
> +out:
>  	return error;
>  }
> 
> @@ -146,6 +204,7 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd)
>  static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
>  {
>  	struct bin_attribute *bin_attr;
> +	struct sysfs_inode_attrs * iattrs;
> 
>  	inode->i_private = sysfs_get(sd);
>  	inode->i_mapping->a_ops = &sysfs_aops;
> @@ -154,16 +213,20 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
>  	inode->i_ino = sd->s_ino;
>  	lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
> 
> -	if (sd->s_iattr) {
> +	iattrs = sd->s_iattr;
> +	if (iattrs) {
>  		/* sysfs_dirent has non-default attributes
>  		 * get them for the new inode from persistent copy
>  		 * in sysfs_dirent
>  		 */
> -		set_inode_attr(inode, sd->s_iattr);
> +		set_inode_attr(inode, &iattrs->ia_iattr);
> +		if (iattrs->ia_secdata)
> +			security_inode_notifysecctx(inode,
> +						iattrs->ia_secdata,
> +						iattrs->ia_secdata_len);
>  	} else
>  		set_default_inode_attr(inode, sd->s_mode);
> 
> -
>  	/* initialize inode according to type */
>  	switch (sysfs_type(sd)) {
>  	case SYSFS_DIR:
> diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> index 1d897ad..c5081ad 100644
> --- a/fs/sysfs/symlink.c
> +++ b/fs/sysfs/symlink.c
> @@ -16,6 +16,7 @@
>  #include <linux/kobject.h>
>  #include <linux/namei.h>
>  #include <linux/mutex.h>
> +#include <linux/security.h>
> 
>  #include "sysfs.h"
> 
> @@ -209,6 +210,7 @@ static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd, void *co
>  }
> 
>  const struct inode_operations sysfs_symlink_inode_operations = {
> +	.setxattr = sysfs_setxattr,
>  	.readlink = generic_readlink,
>  	.follow_link = sysfs_follow_link,
>  	.put_link = sysfs_put_link,
> diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
> index 3fa0d98..af4c4e7 100644
> --- a/fs/sysfs/sysfs.h
> +++ b/fs/sysfs/sysfs.h
> @@ -8,6 +8,8 @@
>   * This file is released under the GPLv2.
>   */
> 
> +#include <linux/fs.h>
> +
>  struct sysfs_open_dirent;
> 
>  /* type-specific structures for sysfs_dirent->s_* union members */
> @@ -31,6 +33,12 @@ struct sysfs_elem_bin_attr {
>  	struct hlist_head	buffers;
>  };
> 
> +struct sysfs_inode_attrs {
> +	struct iattr	ia_iattr;
> +	void		*ia_secdata;
> +	u32		ia_secdata_len;
> +};
> +
>  /*
>   * sysfs_dirent - the building block of sysfs hierarchy.  Each and
>   * every sysfs node is represented by single sysfs_dirent.
> @@ -56,7 +64,7 @@ struct sysfs_dirent {
>  	unsigned int		s_flags;
>  	ino_t			s_ino;
>  	umode_t			s_mode;
> -	struct iattr		*s_iattr;
> +	struct sysfs_inode_attrs *s_iattr;
>  };
> 
>  #define SD_DEACTIVATED_BIAS		INT_MIN
> @@ -148,6 +156,8 @@ static inline void __sysfs_put(struct sysfs_dirent *sd)
>  struct inode *sysfs_get_inode(struct sysfs_dirent *sd);
>  void sysfs_delete_inode(struct inode *inode);
>  int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
> +int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
> +		size_t size, int flags);
>  int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name);
>  int sysfs_inode_init(void);
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f1d5677..38ed894 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -448,6 +448,10 @@ static int sb_finish_set_opts(struct super_block *sb)
>  	    sbsec->behavior > ARRAY_SIZE(labeling_behaviors))
>  		sbsec->flags &= ~SE_SBLABELSUPP;
> 
> +	/* Special handling for sysfs. Is genfs but also has setxattr handler*/
> +	if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0)
> +		sbsec->flags |= SE_SBLABELSUPP;
> +
>  	/* Initialize the root inode. */
>  	rc = inode_doinit_with_dentry(root_inode, root);
> 
> -- 
> 1.5.6.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[James Morris]

This last patch introduced the following warnings:

fs/sysfs/inode.c: In function sysfs_setxattr:
fs/sysfs/inode.c:150: warning: passing argument 2 of 
security_inode_getsecctx from incompatible pointer type
include/linux/security.h:1883: note: expected void ** but argument is of type char ** 
fs/sysfs/inode.c:129: warning: unused variable suffix


Please fix them.  Thanks.



- James
-- 
James Morris
<jmorris@namei.org>

[PATCH 3/3] sysfs: Add labeling support for sysfs

[Stephen Smalley]

From:  David P. Quigley <dpquigl@tycho.nsa.gov>

This patch adds a setxattr handler to the file, directory, and symlink
inode_operations structures for sysfs. The patch uses hooks introduced in the
previous patch to handle the getting and setting of security information for
the sysfs inodes. As was suggested by Eric Biederman the struct iattr in the
sysfs_dirent structure has been replaced by a structure which contains the
iattr, secdata and secdata length to allow the changes to persist in the event
that the inode representing the sysfs_dirent is evicted. Because sysfs only
stores this information when a change is made all the optional data is moved
into one dynamically allocated field.

This patch addresses an issue where SELinux was denying virtd access to the PCI
configuration entries in sysfs. The lack of setxattr handlers for sysfs
required that a single label be assigned to all entries in sysfs. Granting virtd
access to every entry in sysfs is not an acceptable solution so fine grained
labeling of sysfs is required such that individual entries can be labeled
appropriately.

[sds:  Fixed compile-time warnings, coding style, and setting of inode security init flags.]

Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
Signed-off-by: Stephen D. Smalley <sds@tycho.nsa.gov>

---
 fs/sysfs/dir.c             |    1 
 fs/sysfs/inode.c           |  134 ++++++++++++++++++++++++++++++++-------------
 fs/sysfs/symlink.c         |    2 
 fs/sysfs/sysfs.h           |   12 +++-
 security/selinux/hooks.c   |    5 +
 security/smack/smack_lsm.c |    1 
 6 files changed, 118 insertions(+), 37 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 14f2d71..0050fc4 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -760,6 +760,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
 const struct inode_operations sysfs_dir_inode_operations = {
 	.lookup		= sysfs_lookup,
 	.setattr	= sysfs_setattr,
+	.setxattr	= sysfs_setxattr,
 };
 
 static void remove_dir(struct sysfs_dirent *sd)
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 555f0ff..d4f5bda 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -18,6 +18,8 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
+#include <linux/xattr.h>
+#include <linux/security.h>
 #include "sysfs.h"
 
 extern struct super_block * sysfs_sb;
@@ -35,6 +37,7 @@ static struct backing_dev_info sysfs_backing_dev_info = {
 
 static const struct inode_operations sysfs_inode_operations ={
 	.setattr	= sysfs_setattr,
+	.setxattr	= sysfs_setxattr,
 };
 
 int __init sysfs_inode_init(void)
@@ -42,18 +45,37 @@ int __init sysfs_inode_init(void)
 	return bdi_init(&sysfs_backing_dev_info);
 }
 
+struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd)
+{
+	struct sysfs_inode_attrs *attrs;
+	struct iattr *iattrs;
+
+	attrs = kzalloc(sizeof(struct sysfs_inode_attrs), GFP_KERNEL);
+	if (!attrs)
+		return NULL;
+	iattrs = &attrs->ia_iattr;
+
+	/* assign default attributes */
+	iattrs->ia_mode = sd->s_mode;
+	iattrs->ia_uid = 0;
+	iattrs->ia_gid = 0;
+	iattrs->ia_atime = iattrs->ia_mtime = iattrs->ia_ctime = CURRENT_TIME;
+
+	return attrs;
+}
 int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
 {
 	struct inode * inode = dentry->d_inode;
 	struct sysfs_dirent * sd = dentry->d_fsdata;
-	struct iattr * sd_iattr;
+	struct sysfs_inode_attrs *sd_attrs;
+	struct iattr *iattrs;
 	unsigned int ia_valid = iattr->ia_valid;
 	int error;
 
 	if (!sd)
 		return -EINVAL;
 
-	sd_iattr = sd->s_iattr;
+	sd_attrs = sd->s_iattr;
 
 	error = inode_change_ok(inode, iattr);
 	if (error)
@@ -65,42 +87,77 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
 	if (error)
 		return error;
 
-	if (!sd_iattr) {
+	if (!sd_attrs) {
 		/* setting attributes for the first time, allocate now */
-		sd_iattr = kzalloc(sizeof(struct iattr), GFP_KERNEL);
-		if (!sd_iattr)
+		sd_attrs = sysfs_init_inode_attrs(sd);
+		if (!sd_attrs)
 			return -ENOMEM;
-		/* assign default attributes */
-		sd_iattr->ia_mode = sd->s_mode;
-		sd_iattr->ia_uid = 0;
-		sd_iattr->ia_gid = 0;
-		sd_iattr->ia_atime = sd_iattr->ia_mtime = sd_iattr->ia_ctime = CURRENT_TIME;
-		sd->s_iattr = sd_iattr;
+		sd->s_iattr = sd_attrs;
+	} else {
+		/* attributes were changed at least once in past */
+		iattrs = &sd_attrs->ia_iattr;
+
+		if (ia_valid & ATTR_UID)
+			iattrs->ia_uid = iattr->ia_uid;
+		if (ia_valid & ATTR_GID)
+			iattrs->ia_gid = iattr->ia_gid;
+		if (ia_valid & ATTR_ATIME)
+			iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_MTIME)
+			iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_CTIME)
+			iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
+					inode->i_sb->s_time_gran);
+		if (ia_valid & ATTR_MODE) {
+			umode_t mode = iattr->ia_mode;
+
+			if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
+				mode &= ~S_ISGID;
+			iattrs->ia_mode = sd->s_mode = mode;
+		}
 	}
+	return error;
+}
 
-	/* attributes were changed atleast once in past */
-
-	if (ia_valid & ATTR_UID)
-		sd_iattr->ia_uid = iattr->ia_uid;
-	if (ia_valid & ATTR_GID)
-		sd_iattr->ia_gid = iattr->ia_gid;
-	if (ia_valid & ATTR_ATIME)
-		sd_iattr->ia_atime = timespec_trunc(iattr->ia_atime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MTIME)
-		sd_iattr->ia_mtime = timespec_trunc(iattr->ia_mtime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_CTIME)
-		sd_iattr->ia_ctime = timespec_trunc(iattr->ia_ctime,
-						inode->i_sb->s_time_gran);
-	if (ia_valid & ATTR_MODE) {
-		umode_t mode = iattr->ia_mode;
-
-		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
-			mode &= ~S_ISGID;
-		sd_iattr->ia_mode = sd->s_mode = mode;
-	}
+int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags)
+{
+	struct sysfs_dirent *sd = dentry->d_fsdata;
+	struct sysfs_inode_attrs *iattrs;
+	void *secdata;
+	int error;
+	u32 secdata_len = 0;
+
+	if (!sd)
+		return -EINVAL;
+	if (!sd->s_iattr)
+		sd->s_iattr = sysfs_init_inode_attrs(sd);
+	if (!sd->s_iattr)
+		return -ENOMEM;
+
+	iattrs = sd->s_iattr;
+
+	if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
+		const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
+		error = security_inode_setsecurity(dentry->d_inode, suffix,
+						value, size, flags);
+		if (error)
+			goto out;
+		error = security_inode_getsecctx(dentry->d_inode,
+						&secdata, &secdata_len);
+		if (error)
+			goto out;
+		if (iattrs->ia_secdata)
+			security_release_secctx(iattrs->ia_secdata,
+						iattrs->ia_secdata_len);
+		iattrs->ia_secdata = secdata;
+		iattrs->ia_secdata_len = secdata_len;
 
+	} else
+		return -EINVAL;
+out:
 	return error;
 }
 
@@ -146,6 +203,7 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd)
 static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
 {
 	struct bin_attribute *bin_attr;
+	struct sysfs_inode_attrs *iattrs;
 
 	inode->i_private = sysfs_get(sd);
 	inode->i_mapping->a_ops = &sysfs_aops;
@@ -154,16 +212,20 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode)
 	inode->i_ino = sd->s_ino;
 	lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
 
-	if (sd->s_iattr) {
+	iattrs = sd->s_iattr;
+	if (iattrs) {
 		/* sysfs_dirent has non-default attributes
 		 * get them for the new inode from persistent copy
 		 * in sysfs_dirent
 		 */
-		set_inode_attr(inode, sd->s_iattr);
+		set_inode_attr(inode, &iattrs->ia_iattr);
+		if (iattrs->ia_secdata)
+			security_inode_notifysecctx(inode,
+						iattrs->ia_secdata,
+						iattrs->ia_secdata_len);
 	} else
 		set_default_inode_attr(inode, sd->s_mode);
 
-
 	/* initialize inode according to type */
 	switch (sysfs_type(sd)) {
 	case SYSFS_DIR:
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 1d897ad..c5081ad 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -16,6 +16,7 @@
 #include <linux/kobject.h>
 #include <linux/namei.h>
 #include <linux/mutex.h>
+#include <linux/security.h>
 
 #include "sysfs.h"
 
@@ -209,6 +210,7 @@ static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd, void *co
 }
 
 const struct inode_operations sysfs_symlink_inode_operations = {
+	.setxattr = sysfs_setxattr,
 	.readlink = generic_readlink,
 	.follow_link = sysfs_follow_link,
 	.put_link = sysfs_put_link,
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 3fa0d98..af4c4e7 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -8,6 +8,8 @@
  * This file is released under the GPLv2.
  */
 
+#include <linux/fs.h>
+
 struct sysfs_open_dirent;
 
 /* type-specific structures for sysfs_dirent->s_* union members */
@@ -31,6 +33,12 @@ struct sysfs_elem_bin_attr {
 	struct hlist_head	buffers;
 };
 
+struct sysfs_inode_attrs {
+	struct iattr	ia_iattr;
+	void		*ia_secdata;
+	u32		ia_secdata_len;
+};
+
 /*
  * sysfs_dirent - the building block of sysfs hierarchy.  Each and
  * every sysfs node is represented by single sysfs_dirent.
@@ -56,7 +64,7 @@ struct sysfs_dirent {
 	unsigned int		s_flags;
 	ino_t			s_ino;
 	umode_t			s_mode;
-	struct iattr		*s_iattr;
+	struct sysfs_inode_attrs *s_iattr;
 };
 
 #define SD_DEACTIVATED_BIAS		INT_MIN
@@ -148,6 +156,8 @@ static inline void __sysfs_put(struct sysfs_dirent *sd)
 struct inode *sysfs_get_inode(struct sysfs_dirent *sd);
 void sysfs_delete_inode(struct inode *inode);
 int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
+int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags);
 int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name);
 int sysfs_inode_init(void);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7118be2..417f7c9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -448,6 +448,10 @@ static int sb_finish_set_opts(struct super_block *sb)
 	    sbsec->behavior > ARRAY_SIZE(labeling_behaviors))
 		sbsec->flags &= ~SE_SBLABELSUPP;
 
+	/* Special handling for sysfs. Is genfs but also has setxattr handler*/
+	if (strncmp(sb->s_type->name, "sysfs", sizeof("sysfs")) == 0)
+		sbsec->flags |= SE_SBLABELSUPP;
+
 	/* Initialize the root inode. */
 	rc = inode_doinit_with_dentry(root_inode, root);
 
@@ -2923,6 +2927,7 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name,
 		return rc;
 
 	isec->sid = newsid;
+	isec->initialized = 1;
 	return 0;
 }
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 7dc13c3..61df146 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1666,6 +1666,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 
 	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
 		nsp->smk_inode = sp;
+		nsp->smk_flags |= SMK_INODE_INSTANT;
 		return 0;
 	}
 	/*

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[James Morris]

Thanks, all applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Greg KH]

On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
> Thanks, all applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

Um, wait, what about the sysfs maintainer's review?  :)

I also want to see Casey agree with this as well.

thanks,

greg k-h

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Casey Schaufler]

Greg KH wrote:
> On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
>   
>> Thanks, all applied to
>> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>>     
>
> Um, wait, what about the sysfs maintainer's review?  :)
>
> I also want to see Casey agree with this as well.
>   

Without being able to provide a better solution in a reasonable
time I don't see that I can responsibly raise an objection. Sure,
I'd rather see a real implementation of xattrs on memory based
file systems, but I can't put the time in to make it happen and
I don't see that it would be "fair" for these people to be held
up by my petty idiosyncrasies on the topic if I'm not willing to
make the investment and do it myself.

Maybe the next time I'm out of work ("Nuts! And I finally got
asking if they wanted to SuperSize it down!") I'll revive my
efforts to eliminate secids from the kernel entirely.

> thanks,
>
> greg k-h
>
>
>   

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Stephen Smalley]

On Wed, 2009-09-09 at 20:48 -0700, Casey Schaufler wrote:
> Greg KH wrote:
> > On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
> >   
> >> Thanks, all applied to
> >> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
> >>     
> >
> > Um, wait, what about the sysfs maintainer's review?  :)
> >
> > I also want to see Casey agree with this as well.
> >   
> 
> Without being able to provide a better solution in a reasonable
> time I don't see that I can responsibly raise an objection. Sure,
> I'd rather see a real implementation of xattrs on memory based
> file systems, but I can't put the time in to make it happen and
> I don't see that it would be "fair" for these people to be held
> up by my petty idiosyncrasies on the topic if I'm not willing to
> make the investment and do it myself.
> 
> Maybe the next time I'm out of work ("Nuts! And I finally got
> asking if they wanted to SuperSize it down!") I'll revive my
> efforts to eliminate secids from the kernel entirely.

Casey - we reworked the patch to avoid the use of secids in the
interface between the security module and sysfs and to support
multi-xattr security modules (by leveraging the previously discussed
hooks introduced for labeled NFS).  So I believe we went to the trouble
of addressing your concerns.  An Acked-by would be appreciated (and is
necessary at least for the changes to Smack, which we updated for you to
likewise support sysfs labeling).

-- 
Stephen Smalley
National Security Agency

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[James Morris]

On Wed, 9 Sep 2009, Greg KH wrote:

> On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
> > Thanks, all applied to
> > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
> 
> Um, wait, what about the sysfs maintainer's review?  :)

Sorry, I thought it had been reviewed.   Want me to drop it?

> 
> I also want to see Casey agree with this as well.
> 
> thanks,
> 
> greg k-h
> 

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Casey Schaufler]

Greg KH wrote:
> On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
>   
>> Thanks, all applied to
>> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>>     
>
> Um, wait, what about the sysfs maintainer's review?  :)
>
> I also want to see Casey agree with this as well.
>   

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

As I said before, I still want to see general xattr support,
but I suppose that can wait for a later date. I've held this
up long enough.


> thanks,
>
> greg k-h
>
>
>   

Re: [PATCH 3/3] sysfs: Add labeling support for sysfs

[Greg KH]

On Thu, Sep 10, 2009 at 09:17:10PM -0700, Casey Schaufler wrote:
> Greg KH wrote:
> > On Thu, Sep 10, 2009 at 10:40:59AM +1000, James Morris wrote:
> >   
> >> Thanks, all applied to
> >> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
> >>     
> >
> > Um, wait, what about the sysfs maintainer's review?  :)
> >
> > I also want to see Casey agree with this as well.
> >   
> 
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> 
> As I said before, I still want to see general xattr support,
> but I suppose that can wait for a later date. I've held this
> up long enough.

Ok, I have no objections to it either.  James, please add:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
to the patches.

thanks,

greg k-h