[PATCH 3/3] kernfs: Initialize security of newly created nodes

Stephen Smalley sds at tycho.nsa.gov
Wed Jan 9 15:44:29 UTC 2019


On 1/9/19 4:10 AM, Ondrej Mosnacek wrote:
> Use the new security_object_init_security() hook to allow LSMs to
> possibly assign a non-default security context to newly created nodes
> based on the context of their parent node.
> 
> This fixes an issue with cgroupfs under SELinux, where newly created
> cgroup subdirectories would not inherit its parent's context if it had
> been set explicitly to a non-default value (other than the genfs context
> specified by the policy). This can be reproduced as follows:
> 
>      # mkdir /sys/fs/cgroup/unified/test
>      # chcon -R system_u:object_r:cgroup_t:s0:c123 /sys/fs/cgroup/unified/test
>      # ls -lZ /sys/fs/cgroup/unified
>      total 0
>      -r--r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.controllers
>      -rw-r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.max.depth
>      -rw-r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.max.descendants
>      -rw-r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.procs
>      -r--r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.stat
>      -rw-r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.subtree_control
>      -rw-r--r--.  1 root root system_u:object_r:cgroup_t:s0      0 Jan  8 05:00 cgroup.threads
>      drwxr-xr-x.  2 root root system_u:object_r:cgroup_t:s0      0 Jan  8 04:54 init.scope
>      drwxr-xr-x. 25 root root system_u:object_r:cgroup_t:s0      0 Jan  8 04:54 system.slice
>      drwxr-xr-x.  2 root root system_u:object_r:cgroup_t:s0:c123 0 Jan  8 04:59 test
>      drwxr-xr-x.  3 root root system_u:object_r:cgroup_t:s0      0 Jan  8 04:55 user.slice
>      # mkdir /sys/fs/cgroup/unified/test/subdir
> 
> Actual result:
> 
>      # ls -ldZ /sys/fs/cgroup/unified/test/subdir
>      drwxr-xr-x. 2 root root system_u:object_r:cgroup_t:s0 0 Jan  8 05:10 /sys/fs/cgroup/unified/test/subdir
> 
> Expected result:
> 
>      # ls -ldZ /sys/fs/cgroup/unified/test/subdir
>      drwxr-xr-x. 2 root root unconfined_u:object_r:cgroup_t:s0:c123 0 Jan  8 05:10 /sys/fs/cgroup/unified/test/subdir
> 
> Link: https://github.com/SELinuxProject/selinux-kernel/issues/39
> Signed-off-by: Ondrej Mosnacek <omosnace at redhat.com>

Reviewed-by: Stephen Smalley <sds at tycho.nsa.gov>

> ---
>   fs/kernfs/dir.c             | 49 ++++++++++++++++++++++++++++++++++---
>   fs/kernfs/inode.c           |  9 +++----
>   fs/kernfs/kernfs-internal.h |  4 +++
>   3 files changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 4ca0b5c18192..8a678a934f65 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -15,6 +15,7 @@
>   #include <linux/slab.h>
>   #include <linux/security.h>
>   #include <linux/hash.h>
> +#include <linux/stringhash.h>
>   
>   #include "kernfs-internal.h"
>   
> @@ -617,7 +618,43 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry)
>   	return NULL;
>   }
>   
> -static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
> +static int kernfs_node_init_security(struct kernfs_node *parent,
> +				     struct kernfs_node *kn, umode_t mode)
> +{
> +	struct kernfs_iattrs *attrs;
> +	struct qstr q;
> +	void *ctx;
> +	u32 ctxlen;
> +	int ret;
> +
> +	/* If parent has no explicit context set, leave child unset as well */
> +	if (!parent->iattr)
> +		return 0;
> +	if (!parent->iattr->ia_secdata || !parent->iattr->ia_secdata_len)
> +		return 0;
> +
> +	q.name = kn->name;
> +	q.hash_len = hashlen_string(parent, kn->name);
> +
> +	ret = security_object_init_security(parent->iattr->ia_secdata,
> +					    parent->iattr->ia_secdata_len,
> +					    &q, (u16)mode, &ctx, &ctxlen);
> +	if (ret)
> +		return ret;
> +
> +	attrs = kernfs_iattrs(kn);
> +	if (!attrs) {
> +		security_release_secctx(ctx, ctxlen);
> +		return -ENOMEM;
> +	}
> +
> +	kernfs_node_setsecdata(attrs, &ctx, &ctxlen);
> +	/* The inode is fresh, so the returned ctx is always NULL. */
> +	return 0;
> +}
> +
> +static struct kernfs_node *__kernfs_new_node(struct kernfs_node *parent,
> +					     struct kernfs_root *root,
>   					     const char *name, umode_t mode,
>   					     kuid_t uid, kgid_t gid,
>   					     unsigned flags)
> @@ -674,6 +711,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
>   			goto err_out3;
>   	}
>   
> +	if (parent) {
> +		ret = kernfs_node_init_security(parent, kn, mode);
> +		if (ret)
> +			goto err_out3;
> +	}
> +
>   	return kn;
>   
>    err_out3:
> @@ -692,7 +735,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
>   {
>   	struct kernfs_node *kn;
>   
> -	kn = __kernfs_new_node(kernfs_root(parent),
> +	kn = __kernfs_new_node(parent, kernfs_root(parent),
>   			       name, mode, uid, gid, flags);
>   	if (kn) {
>   		kernfs_get(parent);
> @@ -962,7 +1005,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
>   	INIT_LIST_HEAD(&root->supers);
>   	root->next_generation = 1;
>   
> -	kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
> +	kn = __kernfs_new_node(NULL, root, "", S_IFDIR | S_IRUGO | S_IXUGO,
>   			       GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
>   			       KERNFS_DIR);
>   	if (!kn) {
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 80cebcd94c90..e6db8d23437b 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -31,7 +31,7 @@ static const struct inode_operations kernfs_iops = {
>   	.listxattr	= kernfs_iop_listxattr,
>   };
>   
> -static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
> +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn)
>   {
>   	static DEFINE_MUTEX(iattr_mutex);
>   	struct kernfs_iattrs *ret;
> @@ -135,8 +135,8 @@ out:
>   	return error;
>   }
>   
> -static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> -				  u32 *secdata_len)
> +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> +			    u32 *secdata_len)
>   {
>   	void *old_secdata;
>   	size_t old_secdata_len;
> @@ -149,7 +149,6 @@ static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
>   
>   	*secdata = old_secdata;
>   	*secdata_len = old_secdata_len;
> -	return 0;
>   }
>   
>   ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> @@ -365,7 +364,7 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
>   		return error;
>   
>   	mutex_lock(&kernfs_mutex);
> -	error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> +	kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
>   	mutex_unlock(&kernfs_mutex);
>   
>   	if (secdata)
> diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
> index 3d83b114bb08..f6fb2df24c30 100644
> --- a/fs/kernfs/kernfs-internal.h
> +++ b/fs/kernfs/kernfs-internal.h
> @@ -92,6 +92,10 @@ int kernfs_iop_getattr(const struct path *path, struct kstat *stat,
>   ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
>   int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr);
>   
> +struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn);
> +void kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> +			    u32 *secdata_len);
> +
>   /*
>    * dir.c
>    */
> 



More information about the Linux-security-module-archive mailing list