[PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
preparation for NFS to be able to own its own mount options and remove
the NFS specific code from SELinux.

>From the point of view of SELinux NFS does not use text based mount
options so this patch is not something that is going away.  NFS merely
takes the text options from userspace and puts them into the same binary
format as always and passes that binary mount data around.

Signed-off-by: Eric Paris <eparis@redhat.com>

---

 include/linux/security.h          |   70 +++++
 security/dummy.c                  |   26 ++
 security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
 security/selinux/include/objsec.h |    1 +
 4 files changed, 505 insertions(+), 210 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 1a15526..65f2d91 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -34,6 +34,12 @@
 #include <linux/xfrm.h>
 #include <net/flow.h>
 
+/* only a char in selinux superblock security struct flags */
+#define FSCONTEXT_MNT		0x01
+#define CONTEXT_MNT		0x02
+#define ROOTCONTEXT_MNT		0x04
+#define DEFCONTEXT_MNT		0x08
+
 struct ctl_table;
 
 /*
@@ -248,6 +254,22 @@ struct request_sock;
  *	Update module state after a successful pivot.
  *	@old_nd contains the nameidata structure for the old root.
  *      @new_nd contains the nameidata structure for the new root.
+ * @sb_get_mnt_opts:
+ *      Get the security relevant mount options used for a superblock
+ *      @sb the superblock to get security mount options from
+ *      @mount_options array for pointers to mount options
+ *      @mount_flags array of ints specifying what each mount options is
+ *      @num_opts number of options in the arrays
+ * @sb_set_mnt_opts:
+ *      Set the security relevant mount options used for a superblock
+ *      @sb the superblock to set security mount options for
+ *      @mount_options array for pointers to mount options
+ *      @mount_flags array of ints specifying what each mount options is
+ *      @num_opts number of options in the arrays
+ * @sb_clone_mnt_opts:
+ *	Copy all security options from a given superblock to another
+ *	@oldsb old superblock which contain information to clone
+ *	@newsb new superblock which needs filled in
  *
  * Security hooks for inode operations.
  *
@@ -1201,6 +1223,13 @@ struct security_operations {
 			     struct nameidata * new_nd);
 	void (*sb_post_pivotroot) (struct nameidata * old_nd,
 				   struct nameidata * new_nd);
+	int (*sb_get_mnt_opts) (const struct super_block *sb,
+				char ***mount_options, int **flags,
+				int *num_opts);
+	int (*sb_set_mnt_opts) (struct super_block *sb, char **mount_options,
+				int *flags, int num_opts);
+	void (*sb_clone_mnt_opts) (const struct super_block *oldsb,
+				   struct super_block *newsb);
 
 	int (*inode_alloc_security) (struct inode *inode);	
 	void (*inode_free_security) (struct inode *inode);
@@ -1594,6 +1623,26 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
 	security_ops->sb_post_pivotroot (old_nd, new_nd);
 }
 
+static inline int security_sb_get_mnt_opts (const struct super_block *sb,
+					    char ***mount_options,
+					    int **flags, int *num_opts)
+{
+	return security_ops->sb_get_mnt_opts(sb, mount_options, flags, num_opts);
+}
+
+static inline int security_sb_set_mnt_opts (struct super_block *sb,
+					    char **mount_options,
+					    int *flags, int num_opts)
+{
+	return security_ops->sb_set_mnt_opts(sb, mount_options, flags, num_opts);
+}
+
+static inline void security_sb_clone_mnt_opts(const struct super_block *oldsb,
+					      struct super_block *newsb)
+{
+	security_ops->sb_clone_mnt_opts(oldsb, newsb);
+}
+
 static inline int security_inode_alloc (struct inode *inode)
 {
 	inode->i_security = NULL;
@@ -2334,6 +2383,27 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
 					       struct nameidata *new_nd)
 { }
 
+static inline int security_sb_get_mnt_opts (const struct super_block *sb,
+					    char ***mount_options,
+					    int **flags, int *num_opts)
+{
+	*mount_options = NULL;
+	*flags = NULL;
+	*num_opts = 0;
+	return 0;
+}
+
+static inline int security_sb_set_mnt_opts (struct super_block *sb,
+					    char **mount_options,
+					    int *flags, int num_opts)
+{
+	return 0;
+}
+
+static inline void security_sb_clone_mnt_opts(const struct super_block *oldsb,
+					     struct super_block *newsb)
+{ }
+
 static inline int security_inode_alloc (struct inode *inode)
 {
 	return 0;
diff --git a/security/dummy.c b/security/dummy.c
index 853ec22..c3ac361 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -248,6 +248,29 @@ static void dummy_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata
 	return;
 }
 
+static int dummy_sb_get_mnt_opts(const struct super_block *sb, char ***mount_options,
+				 int **flags, int *num_opts)
+{
+	*mount_options = NULL;
+	*flags = NULL;
+	*num_opts = 0;
+	return 0;
+}
+
+static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
+				 int *flags, int num_opts)
+{
+	if (unlikely(num_opts))
+		return -EOPNOTSUPP;
+	return 0;
+}
+
+static void dummy_sb_clone_mnt_opts(const struct super_block *oldsb,
+				    struct super_block *newsb)
+{
+	return;
+}
+
 static int dummy_inode_alloc_security (struct inode *inode)
 {
 	return 0;
@@ -996,6 +1019,9 @@ void security_fixup_ops (struct security_operations *ops)
 	set_to_dummy_if_null(ops, sb_post_addmount);
 	set_to_dummy_if_null(ops, sb_pivotroot);
 	set_to_dummy_if_null(ops, sb_post_pivotroot);
+	set_to_dummy_if_null(ops, sb_get_mnt_opts);
+	set_to_dummy_if_null(ops, sb_set_mnt_opts);
+	set_to_dummy_if_null(ops, sb_clone_mnt_opts);
 	set_to_dummy_if_null(ops, inode_alloc_security);
 	set_to_dummy_if_null(ops, inode_free_security);
 	set_to_dummy_if_null(ops, inode_init_security);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3694662..5ebe30b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
 	return rc;
 }
 
-static int try_context_mount(struct super_block *sb, void *data)
+static int sb_finish_set_opts(struct super_block *sb)
 {
-	char *context = NULL, *defcontext = NULL;
-	char *fscontext = NULL, *rootcontext = NULL;
-	const char *name;
-	u32 sid;
-	int alloc = 0, rc = 0, seen = 0;
-	struct task_security_struct *tsec = current->security;
 	struct superblock_security_struct *sbsec = sb->s_security;
+	struct dentry *root = sb->s_root;
+	struct inode *root_inode = root->d_inode;
+	int rc = 0;
 
-	if (!data)
-		goto out;
+	BUG_ON(!ss_initialized);
 
-	name = sb->s_type->name;
+	if (sbsec->initialized)
+		return 0;
 
-	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
+	if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
+		/* Make sure that the xattr handler exists and that no
+		   error other than -ENODATA is returned by getxattr on
+		   the root directory.  -ENODATA is ok, as this may be
+		   the first boot of the SELinux kernel before we have
+		   assigned xattr values to the filesystem. */
+		if (!root_inode->i_op->getxattr) {
+			printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
+			       "xattr support\n", sb->s_id, sb->s_type->name);
+			rc = -EOPNOTSUPP;
+			goto out;
+		}
+		rc = root_inode->i_op->getxattr(root, XATTR_NAME_SELINUX, NULL, 0);
+		if (rc < 0 && rc != -ENODATA) {
+			if (rc == -EOPNOTSUPP)
+				printk(KERN_WARNING "SELinux: (dev %s, type "
+				       "%s) has no security xattr handler\n",
+				       sb->s_id, sb->s_type->name);
+			else
+				printk(KERN_WARNING "SELinux: (dev %s, type "
+				       "%s) getxattr errno %d\n", sb->s_id,
+				       sb->s_type->name, -rc);
+			goto out;
+		}
+	}
 
-		/* NFS we understand. */
-		if (!strcmp(name, "nfs")) {
-			struct nfs_mount_data *d = data;
+	sbsec->initialized = 1;
 
-			if (d->version <  NFS_MOUNT_VERSION)
-				goto out;
+	if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) {
+		printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n",
+		       sb->s_id, sb->s_type->name);
+	}
+	else {
+		printk(KERN_DEBUG "SELinux: initialized (dev %s, type %s), %s\n",
+		       sb->s_id, sb->s_type->name,
+		       labeling_behaviors[sbsec->behavior-1]);
+	}
 
-			if (d->context[0]) {
-				context = d->context;
-				seen |= Opt_context;
-			}
-		} else
-			goto out;
+	/* Initialize the root inode. */
+	rc = inode_doinit_with_dentry(root_inode, root);
 
-	} else {
-		/* Standard string-based options. */
-		char *p, *options = data;
+	/* Initialize any other inodes associated with the superblock, e.g.
+	   inodes created prior to initial policy load or inodes created
+	   during get_sb by a pseudo filesystem that directly
+	   populates itself. */
+	spin_lock(&sbsec->isec_lock);
+next_inode:
+	if (!list_empty(&sbsec->isec_head)) {
+		struct inode_security_struct *isec =
+				list_entry(sbsec->isec_head.next,
+					   struct inode_security_struct, list);
+		struct inode *inode = isec->inode;
+		spin_unlock(&sbsec->isec_lock);
+		inode = igrab(inode);
+		if (inode) {
+			if (!IS_PRIVATE (inode))
+				inode_doinit(inode);
+			iput(inode);
+		}
+		spin_lock(&sbsec->isec_lock);
+		list_del_init(&isec->list);
+		goto next_inode;
+	}
+	spin_unlock(&sbsec->isec_lock);
+out:
+	return rc;
+}
+
+/*
+ * This function should allow an FS (like NFS) to ask what it's mount security
+ * options were so it can use those later for submounts and the like
+ */
+static int selinux_get_mnt_opts(const struct super_block *sb,
+				char ***mount_options, int **mnt_opts_flags,
+				int *num_opts)
+{
+	int rc = 0, i;
+	struct superblock_security_struct *sbsec = sb->s_security;
+	char *context = NULL;
+	u32 len;
+	/* 
+	 * if we ever use sbsec flags for anything other than tracking mount
+	 * settings this is going to need a mask
+	 */
+	char tmp = sbsec->flags; 
+	*num_opts = 0;
+	*mount_options = NULL;
+	*mnt_opts_flags = NULL;
+
+	for(i = 0; i < 8; i++) {
+		if (tmp & 0x01)
+			(*num_opts)++;
+		tmp >>= 1;
+	}
 
-		while ((p = strsep(&options, "|")) != NULL) {
-			int token;
-			substring_t args[MAX_OPT_ARGS];
+	i = 0;
+	*mount_options = kcalloc(*num_opts, sizeof(char *), GFP_ATOMIC);
+	if (!*mount_options) {
+		rc = -ENOMEM;
+		goto out_free;
+	}
 
-			if (!*p)
-				continue;
+	*mnt_opts_flags = kcalloc(*num_opts, sizeof(int), GFP_ATOMIC);
+	if (!*mnt_opts_flags) {
+		rc = -ENOMEM;
+		goto out_free;
+	}
 
-			token = match_token(p, tokens, args);
+	if (sbsec->flags & FSCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = FSCONTEXT_MNT;
+	}
+	if (sbsec->flags & CONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->mntpoint_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = CONTEXT_MNT;
+	}
+	if (sbsec->flags & DEFCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = DEFCONTEXT_MNT;
+	}
+	if (sbsec->flags & ROOTCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = ROOTCONTEXT_MNT;
+	}
 
-			switch (token) {
-			case Opt_context:
-				if (seen & (Opt_context|Opt_defcontext)) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				context = match_strdup(&args[0]);
-				if (!context) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_context;
-				break;
+	BUG_ON(i != *num_opts);
 
-			case Opt_fscontext:
-				if (seen & Opt_fscontext) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				fscontext = match_strdup(&args[0]);
-				if (!fscontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_fscontext;
-				break;
+	return 0;
 
-			case Opt_rootcontext:
-				if (seen & Opt_rootcontext) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				rootcontext = match_strdup(&args[0]);
-				if (!rootcontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_rootcontext;
-				break;
+out_free:
+	/* don't leak context string if security_sid_to_context had an error */
+	if(*mount_options && i)
+		for (; i > 0; i--)
+			kfree((*mount_options)[i-1]);
+	kfree(*mount_options);
+	*mount_options = NULL;
+	kfree(*mnt_opts_flags);
+	*mnt_opts_flags = NULL;
+	*num_opts = 0;
+	return rc;
+}
 
-			case Opt_defcontext:
-				if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
-					rc = -EINVAL;
-					printk(KERN_WARNING "SELinux:  "
-					       "defcontext option is invalid "
-					       "for this filesystem type\n");
-					goto out_free;
-				}
-				if (seen & (Opt_context|Opt_defcontext)) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				defcontext = match_strdup(&args[0]);
-				if (!defcontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_defcontext;
-				break;
+/*
+ * Allow filesystems with binary mount data to explicitly set mount point labeling.
+ */
+int selinux_set_mnt_opts(struct super_block *sb, char **mount_options,
+				 int *flags, int num_opts)
+{
+	int rc = 0, i;
+	u32 sid;
+	struct task_security_struct *tsec = current->security;
+	struct superblock_security_struct *sbsec = sb->s_security;
+	char *fscontext = NULL, *context = NULL, *rootcontext= NULL;
+	char *defcontext = NULL;
+	const char *name = sb->s_type->name;
 
-			default:
-				rc = -EINVAL;
-				printk(KERN_WARNING "SELinux:  unknown mount "
-				       "option\n");
-				goto out_free;
+	mutex_lock(&sbsec->lock);
 
-			}
-		}
+	if (sbsec->initialized) {
+		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
+			__FUNCTION__);
+		goto out;
 	}
 
-	if (!seen)
+	if (strcmp(sb->s_type->name, "proc") == 0)
+		sbsec->proc = 1;
+
+	/* Determine the labeling behavior to use for this filesystem type. */
+	rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
+	if (rc) {
+		printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n",
+		       __FUNCTION__, sb->s_type->name, rc);
 		goto out;
+	}
 
+	for(i = 0; i < num_opts; i++) {
+		switch(flags[i]) {
+		case FSCONTEXT_MNT:
+			fscontext = mount_options[i];
+			sbsec->flags |= FSCONTEXT_MNT;
+			break;
+		case CONTEXT_MNT:
+			context = mount_options[i];
+			sbsec->flags |= CONTEXT_MNT;
+			break;
+		case ROOTCONTEXT_MNT:
+			rootcontext = mount_options[i];
+			sbsec->flags |= ROOTCONTEXT_MNT;
+			break;
+		case DEFCONTEXT_MNT:
+			defcontext = mount_options[i];
+			sbsec->flags |= DEFCONTEXT_MNT;
+			break;
+		default:
+			rc = -EINVAL;
+			goto out;
+		}
+	}
 	/* sets the context of the superblock for the fs being mounted. */
 	if (fscontext) {
 		rc = security_context_to_sid(fscontext, strlen(fscontext), &sid);
@@ -498,12 +591,12 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       fscontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		rc = may_context_mount_sb_relabel(sid, sbsec, tsec);
 		if (rc)
-			goto out_free;
+			goto out;
 
 		sbsec->sid = sid;
 	}
@@ -519,21 +612,23 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       context, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		if (!fscontext) {
 			rc = may_context_mount_sb_relabel(sid, sbsec, tsec);
 			if (rc)
-				goto out_free;
+				goto out;
 			sbsec->sid = sid;
 		} else {
 			rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 			if (rc)
-				goto out_free;
+				goto out;
 		}
-		sbsec->mntpoint_sid = sid;
+		if (!rootcontext)
+			rootcontext = context;
 
+		sbsec->mntpoint_sid = sid;
 		sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
 	}
 
@@ -545,55 +640,234 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       rootcontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 		if (rc)
-			goto out_free;
+			goto out;
 
 		isec->sid = sid;
 		isec->initialized = 1;
 	}
 
 	if (defcontext) {
+		if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
+			rc = -EINVAL;
+			printk(KERN_WARNING "SELinux: defcontext option is "
+			       "invalid for this filesystem type\n");
+			goto out;
+		}
 		rc = security_context_to_sid(defcontext, strlen(defcontext), &sid);
 		if (rc) {
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       defcontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
-		if (sid == sbsec->def_sid)
-			goto out_free;
-
-		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
-		if (rc)
-			goto out_free;
+		if (sid != sbsec->def_sid) {
+			rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
+			if (rc)
+				goto out;
+		}
 
 		sbsec->def_sid = sid;
 	}
 
-out_free:
-	if (alloc) {
-		kfree(context);
-		kfree(defcontext);
-		kfree(fscontext);
-		kfree(rootcontext);
+	rc = sb_finish_set_opts(sb);
+out:
+	mutex_unlock(&sbsec->lock);
+	return rc;
+}
+
+static void selinux_sb_clone_mnt_opts (const struct super_block *oldsb,
+					struct super_block *newsb)
+{
+	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
+	struct superblock_security_struct *newsbsec = newsb->s_security;
+
+	int set_fscontext =	(oldsbsec->flags & FSCONTEXT_MNT);
+	int set_context =	(oldsbsec->flags & CONTEXT_MNT);
+	int set_rootcontext =	(oldsbsec->flags & ROOTCONTEXT_MNT);
+
+	mutex_lock(&newsbsec->lock);
+
+	newsbsec->flags = oldsbsec->flags;
+
+	newsbsec->sid = oldsbsec->sid;
+	newsbsec->def_sid = oldsbsec->def_sid;
+	newsbsec->behavior = oldsbsec->behavior;
+
+	if (set_context) {
+		u32 sid = oldsbsec->mntpoint_sid;
+
+		if (!set_fscontext)
+			newsbsec->sid = sid;
+		if (!set_rootcontext) {
+			struct inode *newinode = newsb->s_root->d_inode;
+			struct inode_security_struct *newisec = newinode->i_security;
+			newisec->sid = sid;
+		}
+		newsbsec->mntpoint_sid = sid;
+	}
+	if (set_rootcontext) {
+		const struct inode *oldinode = oldsb->s_root->d_inode;
+		const struct inode_security_struct *oldisec = oldinode->i_security;
+		struct inode *newinode = newsb->s_root->d_inode;
+		struct inode_security_struct *newisec = newinode->i_security;
+
+		newisec->sid = oldisec->sid;
 	}
+
+	sb_finish_set_opts(newsb);
+	mutex_unlock(&newsbsec->lock);
+}
+
+/*
+ * string mount options parsing and call to function to set the sbsec
+ */
+static int try_context_mount(struct super_block *sb, void *data)
+{
+	char *context = NULL, *defcontext = NULL;
+	char *fscontext = NULL, *rootcontext = NULL;
+	int rc = 0;
+	char *p, *options = data;
+	/* selinux only know about 4 mount options */
+	char *mnt_opts[4];
+	int mnt_opts_flags[4], num_mnt_opts = 0;
+	int con_from_nfs = 0;
+
+	if (!data)
+		goto out;
+
+	/* with the nfs patch this will become a goto out; */
+	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
+		const char *name = sb->s_type->name;
+		/* NFS we understand. */
+		if (!strcmp(name, "nfs")) {
+			struct nfs_mount_data *d = data;
+
+			if (d->version !=  NFS_MOUNT_VERSION)
+				goto out;
+
+			if (d->context[0])
+				context = d->context;
+			con_from_nfs = 1;
+		} else
+			goto out;
+	} else {
+
+	/* Standard string-based options. */
+	while ((p = strsep(&options, "|")) != NULL) {
+		int token;
+		substring_t args[MAX_OPT_ARGS];
+
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+
+		switch (token) {
+		case Opt_context:
+			if (context || defcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			context = match_strdup(&args[0]);
+			if (!context) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_fscontext:
+			if (fscontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			fscontext = match_strdup(&args[0]);
+			if (!fscontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_rootcontext:
+			if (rootcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			rootcontext = match_strdup(&args[0]);
+			if (!rootcontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_defcontext:
+			if (context || defcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			defcontext = match_strdup(&args[0]);
+			if (!defcontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		default:
+			rc = -EINVAL;
+			printk(KERN_WARNING "SELinux:  unknown mount option\n");
+			goto out_err;
+
+		}
+	}
+	} // else for binary mount data.  delete with nfs patch
+
+	if(fscontext){
+		mnt_opts[num_mnt_opts] = fscontext;
+		mnt_opts_flags[num_mnt_opts++] = FSCONTEXT_MNT;
+	}
+	if(context){
+		mnt_opts[num_mnt_opts] = context;
+		mnt_opts_flags[num_mnt_opts++] = CONTEXT_MNT;
+	}
+	if(rootcontext){
+		mnt_opts[num_mnt_opts] = rootcontext;
+		mnt_opts_flags[num_mnt_opts++] = ROOTCONTEXT_MNT;
+	}
+	if(defcontext){
+		mnt_opts[num_mnt_opts] = defcontext;
+		mnt_opts_flags[num_mnt_opts++] = DEFCONTEXT_MNT;
+	}
+
 out:
+	rc = selinux_set_mnt_opts(sb, mnt_opts, mnt_opts_flags, num_mnt_opts);
+out_err:
+	/* remove conditional when we remove nfs */
+	if (!con_from_nfs)
+		kfree(context);
+	kfree(defcontext);
+	kfree(fscontext);
+	kfree(rootcontext);
 	return rc;
 }
 
+/* 
+ * no locking done here, but it shouldn't matter, sbsec->proc would just get
+ * set the same way in another thread and fs_use, hmmmmmm 
+ */
 static int superblock_doinit(struct super_block *sb, void *data)
 {
 	struct superblock_security_struct *sbsec = sb->s_security;
-	struct dentry *root = sb->s_root;
-	struct inode *inode = root->d_inode;
 	int rc = 0;
 
-	mutex_lock(&sbsec->lock);
 	if (sbsec->initialized)
 		goto out;
 
@@ -608,87 +882,8 @@ static int superblock_doinit(struct super_block *sb, void *data)
 		goto out;
 	}
 
-	/* Determine the labeling behavior to use for this filesystem type. */
-	rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
-	if (rc) {
-		printk(KERN_WARNING "%s:  security_fs_use(%s) returned %d\n",
-		       __FUNCTION__, sb->s_type->name, rc);
-		goto out;
-	}
-
 	rc = try_context_mount(sb, data);
-	if (rc)
-		goto out;
-
-	if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
-		/* Make sure that the xattr handler exists and that no
-		   error other than -ENODATA is returned by getxattr on
-		   the root directory.  -ENODATA is ok, as this may be
-		   the first boot of the SELinux kernel before we have
-		   assigned xattr values to the filesystem. */
-		if (!inode->i_op->getxattr) {
-			printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
-			       "xattr support\n", sb->s_id, sb->s_type->name);
-			rc = -EOPNOTSUPP;
-			goto out;
-		}
-		rc = inode->i_op->getxattr(root, XATTR_NAME_SELINUX, NULL, 0);
-		if (rc < 0 && rc != -ENODATA) {
-			if (rc == -EOPNOTSUPP)
-				printk(KERN_WARNING "SELinux: (dev %s, type "
-				       "%s) has no security xattr handler\n",
-				       sb->s_id, sb->s_type->name);
-			else
-				printk(KERN_WARNING "SELinux: (dev %s, type "
-				       "%s) getxattr errno %d\n", sb->s_id,
-				       sb->s_type->name, -rc);
-			goto out;
-		}
-	}
-
-	if (strcmp(sb->s_type->name, "proc") == 0)
-		sbsec->proc = 1;
-
-	sbsec->initialized = 1;
-
-	if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) {
-		printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n",
-		       sb->s_id, sb->s_type->name);
-	}
-	else {
-		printk(KERN_DEBUG "SELinux: initialized (dev %s, type %s), %s\n",
-		       sb->s_id, sb->s_type->name,
-		       labeling_behaviors[sbsec->behavior-1]);
-	}
-
-	/* Initialize the root inode. */
-	rc = inode_doinit_with_dentry(sb->s_root->d_inode, sb->s_root);
-
-	/* Initialize any other inodes associated with the superblock, e.g.
-	   inodes created prior to initial policy load or inodes created
-	   during get_sb by a pseudo filesystem that directly
-	   populates itself. */
-	spin_lock(&sbsec->isec_lock);
-next_inode:
-	if (!list_empty(&sbsec->isec_head)) {
-		struct inode_security_struct *isec =
-				list_entry(sbsec->isec_head.next,
-				           struct inode_security_struct, list);
-		struct inode *inode = isec->inode;
-		spin_unlock(&sbsec->isec_lock);
-		inode = igrab(inode);
-		if (inode) {
-			if (!IS_PRIVATE (inode))
-				inode_doinit(inode);
-			iput(inode);
-		}
-		spin_lock(&sbsec->isec_lock);
-		list_del_init(&isec->list);
-		goto next_inode;
-	}
-	spin_unlock(&sbsec->isec_lock);
 out:
-	mutex_unlock(&sbsec->lock);
 	return rc;
 }
 
@@ -4749,6 +4944,9 @@ static struct security_operations selinux_ops = {
 	.sb_statfs =			selinux_sb_statfs,
 	.sb_mount =			selinux_mount,
 	.sb_umount =			selinux_umount,
+	.sb_get_mnt_opts =		selinux_get_mnt_opts,
+	.sb_set_mnt_opts =		selinux_set_mnt_opts,
+	.sb_clone_mnt_opts = 		selinux_sb_clone_mnt_opts,
 
 	.inode_alloc_security =		selinux_inode_alloc_security,
 	.inode_free_security =		selinux_inode_free_security,
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 91b88f0..fc4768e 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -63,6 +63,7 @@ struct superblock_security_struct {
 	u32 mntpoint_sid;		/* SECURITY_FS_USE_MNTPOINT context for files */
 	unsigned int behavior;          /* labeling behavior */
 	unsigned char initialized;      /* initialization flag */
+	unsigned char flags;		/* which mount options were specified */
 	unsigned char proc;             /* proc fs */
 	struct mutex lock;
 	struct list_head isec_head;



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
preparation for NFS to be able to own its own mount options and remove
the NFS specific code from SELinux.

>From the point of view of SELinux NFS does not use text based mount
options so this patch is not something that is going away.  NFS merely
takes the text options from userspace and puts them into the same binary
format as always and passes that binary mount data around.

Signed-off-by: Eric Paris <eparis@redhat.com>

---

 include/linux/security.h          |   70 +++++
 security/dummy.c                  |   26 ++
 security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
 security/selinux/include/objsec.h |    1 +
 4 files changed, 505 insertions(+), 210 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 1a15526..65f2d91 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -34,6 +34,12 @@
 #include <linux/xfrm.h>
 #include <net/flow.h>
 
+/* only a char in selinux superblock security struct flags */
+#define FSCONTEXT_MNT		0x01
+#define CONTEXT_MNT		0x02
+#define ROOTCONTEXT_MNT		0x04
+#define DEFCONTEXT_MNT		0x08
+
 struct ctl_table;
 
 /*
@@ -248,6 +254,22 @@ struct request_sock;
  *	Update module state after a successful pivot.
  *	@old_nd contains the nameidata structure for the old root.
  *      @new_nd contains the nameidata structure for the new root.
+ * @sb_get_mnt_opts:
+ *      Get the security relevant mount options used for a superblock
+ *      @sb the superblock to get security mount options from
+ *      @mount_options array for pointers to mount options
+ *      @mount_flags array of ints specifying what each mount options is
+ *      @num_opts number of options in the arrays
+ * @sb_set_mnt_opts:
+ *      Set the security relevant mount options used for a superblock
+ *      @sb the superblock to set security mount options for
+ *      @mount_options array for pointers to mount options
+ *      @mount_flags array of ints specifying what each mount options is
+ *      @num_opts number of options in the arrays
+ * @sb_clone_mnt_opts:
+ *	Copy all security options from a given superblock to another
+ *	@oldsb old superblock which contain information to clone
+ *	@newsb new superblock which needs filled in
  *
  * Security hooks for inode operations.
  *
@@ -1201,6 +1223,13 @@ struct security_operations {
 			     struct nameidata * new_nd);
 	void (*sb_post_pivotroot) (struct nameidata * old_nd,
 				   struct nameidata * new_nd);
+	int (*sb_get_mnt_opts) (const struct super_block *sb,
+				char ***mount_options, int **flags,
+				int *num_opts);
+	int (*sb_set_mnt_opts) (struct super_block *sb, char **mount_options,
+				int *flags, int num_opts);
+	void (*sb_clone_mnt_opts) (const struct super_block *oldsb,
+				   struct super_block *newsb);
 
 	int (*inode_alloc_security) (struct inode *inode);	
 	void (*inode_free_security) (struct inode *inode);
@@ -1594,6 +1623,26 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
 	security_ops->sb_post_pivotroot (old_nd, new_nd);
 }
 
+static inline int security_sb_get_mnt_opts (const struct super_block *sb,
+					    char ***mount_options,
+					    int **flags, int *num_opts)
+{
+	return security_ops->sb_get_mnt_opts(sb, mount_options, flags, num_opts);
+}
+
+static inline int security_sb_set_mnt_opts (struct super_block *sb,
+					    char **mount_options,
+					    int *flags, int num_opts)
+{
+	return security_ops->sb_set_mnt_opts(sb, mount_options, flags, num_opts);
+}
+
+static inline void security_sb_clone_mnt_opts(const struct super_block *oldsb,
+					      struct super_block *newsb)
+{
+	security_ops->sb_clone_mnt_opts(oldsb, newsb);
+}
+
 static inline int security_inode_alloc (struct inode *inode)
 {
 	inode->i_security = NULL;
@@ -2334,6 +2383,27 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
 					       struct nameidata *new_nd)
 { }
 
+static inline int security_sb_get_mnt_opts (const struct super_block *sb,
+					    char ***mount_options,
+					    int **flags, int *num_opts)
+{
+	*mount_options = NULL;
+	*flags = NULL;
+	*num_opts = 0;
+	return 0;
+}
+
+static inline int security_sb_set_mnt_opts (struct super_block *sb,
+					    char **mount_options,
+					    int *flags, int num_opts)
+{
+	return 0;
+}
+
+static inline void security_sb_clone_mnt_opts(const struct super_block *oldsb,
+					     struct super_block *newsb)
+{ }
+
 static inline int security_inode_alloc (struct inode *inode)
 {
 	return 0;
diff --git a/security/dummy.c b/security/dummy.c
index 853ec22..c3ac361 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -248,6 +248,29 @@ static void dummy_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata
 	return;
 }
 
+static int dummy_sb_get_mnt_opts(const struct super_block *sb, char ***mount_options,
+				 int **flags, int *num_opts)
+{
+	*mount_options = NULL;
+	*flags = NULL;
+	*num_opts = 0;
+	return 0;
+}
+
+static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
+				 int *flags, int num_opts)
+{
+	if (unlikely(num_opts))
+		return -EOPNOTSUPP;
+	return 0;
+}
+
+static void dummy_sb_clone_mnt_opts(const struct super_block *oldsb,
+				    struct super_block *newsb)
+{
+	return;
+}
+
 static int dummy_inode_alloc_security (struct inode *inode)
 {
 	return 0;
@@ -996,6 +1019,9 @@ void security_fixup_ops (struct security_operations *ops)
 	set_to_dummy_if_null(ops, sb_post_addmount);
 	set_to_dummy_if_null(ops, sb_pivotroot);
 	set_to_dummy_if_null(ops, sb_post_pivotroot);
+	set_to_dummy_if_null(ops, sb_get_mnt_opts);
+	set_to_dummy_if_null(ops, sb_set_mnt_opts);
+	set_to_dummy_if_null(ops, sb_clone_mnt_opts);
 	set_to_dummy_if_null(ops, inode_alloc_security);
 	set_to_dummy_if_null(ops, inode_free_security);
 	set_to_dummy_if_null(ops, inode_init_security);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3694662..5ebe30b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
 	return rc;
 }
 
-static int try_context_mount(struct super_block *sb, void *data)
+static int sb_finish_set_opts(struct super_block *sb)
 {
-	char *context = NULL, *defcontext = NULL;
-	char *fscontext = NULL, *rootcontext = NULL;
-	const char *name;
-	u32 sid;
-	int alloc = 0, rc = 0, seen = 0;
-	struct task_security_struct *tsec = current->security;
 	struct superblock_security_struct *sbsec = sb->s_security;
+	struct dentry *root = sb->s_root;
+	struct inode *root_inode = root->d_inode;
+	int rc = 0;
 
-	if (!data)
-		goto out;
+	BUG_ON(!ss_initialized);
 
-	name = sb->s_type->name;
+	if (sbsec->initialized)
+		return 0;
 
-	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
+	if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
+		/* Make sure that the xattr handler exists and that no
+		   error other than -ENODATA is returned by getxattr on
+		   the root directory.  -ENODATA is ok, as this may be
+		   the first boot of the SELinux kernel before we have
+		   assigned xattr values to the filesystem. */
+		if (!root_inode->i_op->getxattr) {
+			printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
+			       "xattr support\n", sb->s_id, sb->s_type->name);
+			rc = -EOPNOTSUPP;
+			goto out;
+		}
+		rc = root_inode->i_op->getxattr(root, XATTR_NAME_SELINUX, NULL, 0);
+		if (rc < 0 && rc != -ENODATA) {
+			if (rc == -EOPNOTSUPP)
+				printk(KERN_WARNING "SELinux: (dev %s, type "
+				       "%s) has no security xattr handler\n",
+				       sb->s_id, sb->s_type->name);
+			else
+				printk(KERN_WARNING "SELinux: (dev %s, type "
+				       "%s) getxattr errno %d\n", sb->s_id,
+				       sb->s_type->name, -rc);
+			goto out;
+		}
+	}
 
-		/* NFS we understand. */
-		if (!strcmp(name, "nfs")) {
-			struct nfs_mount_data *d = data;
+	sbsec->initialized = 1;
 
-			if (d->version <  NFS_MOUNT_VERSION)
-				goto out;
+	if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) {
+		printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n",
+		       sb->s_id, sb->s_type->name);
+	}
+	else {
+		printk(KERN_DEBUG "SELinux: initialized (dev %s, type %s), %s\n",
+		       sb->s_id, sb->s_type->name,
+		       labeling_behaviors[sbsec->behavior-1]);
+	}
 
-			if (d->context[0]) {
-				context = d->context;
-				seen |= Opt_context;
-			}
-		} else
-			goto out;
+	/* Initialize the root inode. */
+	rc = inode_doinit_with_dentry(root_inode, root);
 
-	} else {
-		/* Standard string-based options. */
-		char *p, *options = data;
+	/* Initialize any other inodes associated with the superblock, e.g.
+	   inodes created prior to initial policy load or inodes created
+	   during get_sb by a pseudo filesystem that directly
+	   populates itself. */
+	spin_lock(&sbsec->isec_lock);
+next_inode:
+	if (!list_empty(&sbsec->isec_head)) {
+		struct inode_security_struct *isec =
+				list_entry(sbsec->isec_head.next,
+					   struct inode_security_struct, list);
+		struct inode *inode = isec->inode;
+		spin_unlock(&sbsec->isec_lock);
+		inode = igrab(inode);
+		if (inode) {
+			if (!IS_PRIVATE (inode))
+				inode_doinit(inode);
+			iput(inode);
+		}
+		spin_lock(&sbsec->isec_lock);
+		list_del_init(&isec->list);
+		goto next_inode;
+	}
+	spin_unlock(&sbsec->isec_lock);
+out:
+	return rc;
+}
+
+/*
+ * This function should allow an FS (like NFS) to ask what it's mount security
+ * options were so it can use those later for submounts and the like
+ */
+static int selinux_get_mnt_opts(const struct super_block *sb,
+				char ***mount_options, int **mnt_opts_flags,
+				int *num_opts)
+{
+	int rc = 0, i;
+	struct superblock_security_struct *sbsec = sb->s_security;
+	char *context = NULL;
+	u32 len;
+	/* 
+	 * if we ever use sbsec flags for anything other than tracking mount
+	 * settings this is going to need a mask
+	 */
+	char tmp = sbsec->flags; 
+	*num_opts = 0;
+	*mount_options = NULL;
+	*mnt_opts_flags = NULL;
+
+	for(i = 0; i < 8; i++) {
+		if (tmp & 0x01)
+			(*num_opts)++;
+		tmp >>= 1;
+	}
 
-		while ((p = strsep(&options, "|")) != NULL) {
-			int token;
-			substring_t args[MAX_OPT_ARGS];
+	i = 0;
+	*mount_options = kcalloc(*num_opts, sizeof(char *), GFP_ATOMIC);
+	if (!*mount_options) {
+		rc = -ENOMEM;
+		goto out_free;
+	}
 
-			if (!*p)
-				continue;
+	*mnt_opts_flags = kcalloc(*num_opts, sizeof(int), GFP_ATOMIC);
+	if (!*mnt_opts_flags) {
+		rc = -ENOMEM;
+		goto out_free;
+	}
 
-			token = match_token(p, tokens, args);
+	if (sbsec->flags & FSCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = FSCONTEXT_MNT;
+	}
+	if (sbsec->flags & CONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->mntpoint_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = CONTEXT_MNT;
+	}
+	if (sbsec->flags & DEFCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = DEFCONTEXT_MNT;
+	}
+	if (sbsec->flags & ROOTCONTEXT_MNT) {
+		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
+		if (rc)
+			goto out_free;
+		(*mount_options)[i] = context;
+		(*mnt_opts_flags)[i++] = ROOTCONTEXT_MNT;
+	}
 
-			switch (token) {
-			case Opt_context:
-				if (seen & (Opt_context|Opt_defcontext)) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				context = match_strdup(&args[0]);
-				if (!context) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_context;
-				break;
+	BUG_ON(i != *num_opts);
 
-			case Opt_fscontext:
-				if (seen & Opt_fscontext) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				fscontext = match_strdup(&args[0]);
-				if (!fscontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_fscontext;
-				break;
+	return 0;
 
-			case Opt_rootcontext:
-				if (seen & Opt_rootcontext) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				rootcontext = match_strdup(&args[0]);
-				if (!rootcontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_rootcontext;
-				break;
+out_free:
+	/* don't leak context string if security_sid_to_context had an error */
+	if(*mount_options && i)
+		for (; i > 0; i--)
+			kfree((*mount_options)[i-1]);
+	kfree(*mount_options);
+	*mount_options = NULL;
+	kfree(*mnt_opts_flags);
+	*mnt_opts_flags = NULL;
+	*num_opts = 0;
+	return rc;
+}
 
-			case Opt_defcontext:
-				if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
-					rc = -EINVAL;
-					printk(KERN_WARNING "SELinux:  "
-					       "defcontext option is invalid "
-					       "for this filesystem type\n");
-					goto out_free;
-				}
-				if (seen & (Opt_context|Opt_defcontext)) {
-					rc = -EINVAL;
-					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
-					goto out_free;
-				}
-				defcontext = match_strdup(&args[0]);
-				if (!defcontext) {
-					rc = -ENOMEM;
-					goto out_free;
-				}
-				if (!alloc)
-					alloc = 1;
-				seen |= Opt_defcontext;
-				break;
+/*
+ * Allow filesystems with binary mount data to explicitly set mount point labeling.
+ */
+int selinux_set_mnt_opts(struct super_block *sb, char **mount_options,
+				 int *flags, int num_opts)
+{
+	int rc = 0, i;
+	u32 sid;
+	struct task_security_struct *tsec = current->security;
+	struct superblock_security_struct *sbsec = sb->s_security;
+	char *fscontext = NULL, *context = NULL, *rootcontext= NULL;
+	char *defcontext = NULL;
+	const char *name = sb->s_type->name;
 
-			default:
-				rc = -EINVAL;
-				printk(KERN_WARNING "SELinux:  unknown mount "
-				       "option\n");
-				goto out_free;
+	mutex_lock(&sbsec->lock);
 
-			}
-		}
+	if (sbsec->initialized) {
+		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
+			__FUNCTION__);
+		goto out;
 	}
 
-	if (!seen)
+	if (strcmp(sb->s_type->name, "proc") == 0)
+		sbsec->proc = 1;
+
+	/* Determine the labeling behavior to use for this filesystem type. */
+	rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
+	if (rc) {
+		printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n",
+		       __FUNCTION__, sb->s_type->name, rc);
 		goto out;
+	}
 
+	for(i = 0; i < num_opts; i++) {
+		switch(flags[i]) {
+		case FSCONTEXT_MNT:
+			fscontext = mount_options[i];
+			sbsec->flags |= FSCONTEXT_MNT;
+			break;
+		case CONTEXT_MNT:
+			context = mount_options[i];
+			sbsec->flags |= CONTEXT_MNT;
+			break;
+		case ROOTCONTEXT_MNT:
+			rootcontext = mount_options[i];
+			sbsec->flags |= ROOTCONTEXT_MNT;
+			break;
+		case DEFCONTEXT_MNT:
+			defcontext = mount_options[i];
+			sbsec->flags |= DEFCONTEXT_MNT;
+			break;
+		default:
+			rc = -EINVAL;
+			goto out;
+		}
+	}
 	/* sets the context of the superblock for the fs being mounted. */
 	if (fscontext) {
 		rc = security_context_to_sid(fscontext, strlen(fscontext), &sid);
@@ -498,12 +591,12 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       fscontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		rc = may_context_mount_sb_relabel(sid, sbsec, tsec);
 		if (rc)
-			goto out_free;
+			goto out;
 
 		sbsec->sid = sid;
 	}
@@ -519,21 +612,23 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       context, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		if (!fscontext) {
 			rc = may_context_mount_sb_relabel(sid, sbsec, tsec);
 			if (rc)
-				goto out_free;
+				goto out;
 			sbsec->sid = sid;
 		} else {
 			rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 			if (rc)
-				goto out_free;
+				goto out;
 		}
-		sbsec->mntpoint_sid = sid;
+		if (!rootcontext)
+			rootcontext = context;
 
+		sbsec->mntpoint_sid = sid;
 		sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
 	}
 
@@ -545,55 +640,234 @@ static int try_context_mount(struct super_block *sb, void *data)
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       rootcontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
 		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 		if (rc)
-			goto out_free;
+			goto out;
 
 		isec->sid = sid;
 		isec->initialized = 1;
 	}
 
 	if (defcontext) {
+		if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
+			rc = -EINVAL;
+			printk(KERN_WARNING "SELinux: defcontext option is "
+			       "invalid for this filesystem type\n");
+			goto out;
+		}
 		rc = security_context_to_sid(defcontext, strlen(defcontext), &sid);
 		if (rc) {
 			printk(KERN_WARNING "SELinux: security_context_to_sid"
 			       "(%s) failed for (dev %s, type %s) errno=%d\n",
 			       defcontext, sb->s_id, name, rc);
-			goto out_free;
+			goto out;
 		}
 
-		if (sid == sbsec->def_sid)
-			goto out_free;
-
-		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
-		if (rc)
-			goto out_free;
+		if (sid != sbsec->def_sid) {
+			rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
+			if (rc)
+				goto out;
+		}
 
 		sbsec->def_sid = sid;
 	}
 
-out_free:
-	if (alloc) {
-		kfree(context);
-		kfree(defcontext);
-		kfree(fscontext);
-		kfree(rootcontext);
+	rc = sb_finish_set_opts(sb);
+out:
+	mutex_unlock(&sbsec->lock);
+	return rc;
+}
+
+static void selinux_sb_clone_mnt_opts (const struct super_block *oldsb,
+					struct super_block *newsb)
+{
+	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
+	struct superblock_security_struct *newsbsec = newsb->s_security;
+
+	int set_fscontext =	(oldsbsec->flags & FSCONTEXT_MNT);
+	int set_context =	(oldsbsec->flags & CONTEXT_MNT);
+	int set_rootcontext =	(oldsbsec->flags & ROOTCONTEXT_MNT);
+
+	mutex_lock(&newsbsec->lock);
+
+	newsbsec->flags = oldsbsec->flags;
+
+	newsbsec->sid = oldsbsec->sid;
+	newsbsec->def_sid = oldsbsec->def_sid;
+	newsbsec->behavior = oldsbsec->behavior;
+
+	if (set_context) {
+		u32 sid = oldsbsec->mntpoint_sid;
+
+		if (!set_fscontext)
+			newsbsec->sid = sid;
+		if (!set_rootcontext) {
+			struct inode *newinode = newsb->s_root->d_inode;
+			struct inode_security_struct *newisec = newinode->i_security;
+			newisec->sid = sid;
+		}
+		newsbsec->mntpoint_sid = sid;
+	}
+	if (set_rootcontext) {
+		const struct inode *oldinode = oldsb->s_root->d_inode;
+		const struct inode_security_struct *oldisec = oldinode->i_security;
+		struct inode *newinode = newsb->s_root->d_inode;
+		struct inode_security_struct *newisec = newinode->i_security;
+
+		newisec->sid = oldisec->sid;
 	}
+
+	sb_finish_set_opts(newsb);
+	mutex_unlock(&newsbsec->lock);
+}
+
+/*
+ * string mount options parsing and call to function to set the sbsec
+ */
+static int try_context_mount(struct super_block *sb, void *data)
+{
+	char *context = NULL, *defcontext = NULL;
+	char *fscontext = NULL, *rootcontext = NULL;
+	int rc = 0;
+	char *p, *options = data;
+	/* selinux only know about 4 mount options */
+	char *mnt_opts[4];
+	int mnt_opts_flags[4], num_mnt_opts = 0;
+	int con_from_nfs = 0;
+
+	if (!data)
+		goto out;
+
+	/* with the nfs patch this will become a goto out; */
+	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
+		const char *name = sb->s_type->name;
+		/* NFS we understand. */
+		if (!strcmp(name, "nfs")) {
+			struct nfs_mount_data *d = data;
+
+			if (d->version !=  NFS_MOUNT_VERSION)
+				goto out;
+
+			if (d->context[0])
+				context = d->context;
+			con_from_nfs = 1;
+		} else
+			goto out;
+	} else {
+
+	/* Standard string-based options. */
+	while ((p = strsep(&options, "|")) != NULL) {
+		int token;
+		substring_t args[MAX_OPT_ARGS];
+
+		if (!*p)
+			continue;
+
+		token = match_token(p, tokens, args);
+
+		switch (token) {
+		case Opt_context:
+			if (context || defcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			context = match_strdup(&args[0]);
+			if (!context) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_fscontext:
+			if (fscontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			fscontext = match_strdup(&args[0]);
+			if (!fscontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_rootcontext:
+			if (rootcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			rootcontext = match_strdup(&args[0]);
+			if (!rootcontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		case Opt_defcontext:
+			if (context || defcontext) {
+				rc = -EINVAL;
+				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+				goto out_err;
+			}
+			defcontext = match_strdup(&args[0]);
+			if (!defcontext) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+			break;
+
+		default:
+			rc = -EINVAL;
+			printk(KERN_WARNING "SELinux:  unknown mount option\n");
+			goto out_err;
+
+		}
+	}
+	} // else for binary mount data.  delete with nfs patch
+
+	if(fscontext){
+		mnt_opts[num_mnt_opts] = fscontext;
+		mnt_opts_flags[num_mnt_opts++] = FSCONTEXT_MNT;
+	}
+	if(context){
+		mnt_opts[num_mnt_opts] = context;
+		mnt_opts_flags[num_mnt_opts++] = CONTEXT_MNT;
+	}
+	if(rootcontext){
+		mnt_opts[num_mnt_opts] = rootcontext;
+		mnt_opts_flags[num_mnt_opts++] = ROOTCONTEXT_MNT;
+	}
+	if(defcontext){
+		mnt_opts[num_mnt_opts] = defcontext;
+		mnt_opts_flags[num_mnt_opts++] = DEFCONTEXT_MNT;
+	}
+
 out:
+	rc = selinux_set_mnt_opts(sb, mnt_opts, mnt_opts_flags, num_mnt_opts);
+out_err:
+	/* remove conditional when we remove nfs */
+	if (!con_from_nfs)
+		kfree(context);
+	kfree(defcontext);
+	kfree(fscontext);
+	kfree(rootcontext);
 	return rc;
 }
 
+/* 
+ * no locking done here, but it shouldn't matter, sbsec->proc would just get
+ * set the same way in another thread and fs_use, hmmmmmm 
+ */
 static int superblock_doinit(struct super_block *sb, void *data)
 {
 	struct superblock_security_struct *sbsec = sb->s_security;
-	struct dentry *root = sb->s_root;
-	struct inode *inode = root->d_inode;
 	int rc = 0;
 
-	mutex_lock(&sbsec->lock);
 	if (sbsec->initialized)
 		goto out;
 
@@ -608,87 +882,8 @@ static int superblock_doinit(struct super_block *sb, void *data)
 		goto out;
 	}
 
-	/* Determine the labeling behavior to use for this filesystem type. */
-	rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
-	if (rc) {
-		printk(KERN_WARNING "%s:  security_fs_use(%s) returned %d\n",
-		       __FUNCTION__, sb->s_type->name, rc);
-		goto out;
-	}
-
 	rc = try_context_mount(sb, data);
-	if (rc)
-		goto out;
-
-	if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
-		/* Make sure that the xattr handler exists and that no
-		   error other than -ENODATA is returned by getxattr on
-		   the root directory.  -ENODATA is ok, as this may be
-		   the first boot of the SELinux kernel before we have
-		   assigned xattr values to the filesystem. */
-		if (!inode->i_op->getxattr) {
-			printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
-			       "xattr support\n", sb->s_id, sb->s_type->name);
-			rc = -EOPNOTSUPP;
-			goto out;
-		}
-		rc = inode->i_op->getxattr(root, XATTR_NAME_SELINUX, NULL, 0);
-		if (rc < 0 && rc != -ENODATA) {
-			if (rc == -EOPNOTSUPP)
-				printk(KERN_WARNING "SELinux: (dev %s, type "
-				       "%s) has no security xattr handler\n",
-				       sb->s_id, sb->s_type->name);
-			else
-				printk(KERN_WARNING "SELinux: (dev %s, type "
-				       "%s) getxattr errno %d\n", sb->s_id,
-				       sb->s_type->name, -rc);
-			goto out;
-		}
-	}
-
-	if (strcmp(sb->s_type->name, "proc") == 0)
-		sbsec->proc = 1;
-
-	sbsec->initialized = 1;
-
-	if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors)) {
-		printk(KERN_ERR "SELinux: initialized (dev %s, type %s), unknown behavior\n",
-		       sb->s_id, sb->s_type->name);
-	}
-	else {
-		printk(KERN_DEBUG "SELinux: initialized (dev %s, type %s), %s\n",
-		       sb->s_id, sb->s_type->name,
-		       labeling_behaviors[sbsec->behavior-1]);
-	}
-
-	/* Initialize the root inode. */
-	rc = inode_doinit_with_dentry(sb->s_root->d_inode, sb->s_root);
-
-	/* Initialize any other inodes associated with the superblock, e.g.
-	   inodes created prior to initial policy load or inodes created
-	   during get_sb by a pseudo filesystem that directly
-	   populates itself. */
-	spin_lock(&sbsec->isec_lock);
-next_inode:
-	if (!list_empty(&sbsec->isec_head)) {
-		struct inode_security_struct *isec =
-				list_entry(sbsec->isec_head.next,
-				           struct inode_security_struct, list);
-		struct inode *inode = isec->inode;
-		spin_unlock(&sbsec->isec_lock);
-		inode = igrab(inode);
-		if (inode) {
-			if (!IS_PRIVATE (inode))
-				inode_doinit(inode);
-			iput(inode);
-		}
-		spin_lock(&sbsec->isec_lock);
-		list_del_init(&isec->list);
-		goto next_inode;
-	}
-	spin_unlock(&sbsec->isec_lock);
 out:
-	mutex_unlock(&sbsec->lock);
 	return rc;
 }
 
@@ -4749,6 +4944,9 @@ static struct security_operations selinux_ops = {
 	.sb_statfs =			selinux_sb_statfs,
 	.sb_mount =			selinux_mount,
 	.sb_umount =			selinux_umount,
+	.sb_get_mnt_opts =		selinux_get_mnt_opts,
+	.sb_set_mnt_opts =		selinux_set_mnt_opts,
+	.sb_clone_mnt_opts = 		selinux_sb_clone_mnt_opts,
 
 	.inode_alloc_security =		selinux_inode_alloc_security,
 	.inode_free_security =		selinux_inode_free_security,
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 91b88f0..fc4768e 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -63,6 +63,7 @@ struct superblock_security_struct {
 	u32 mntpoint_sid;		/* SECURITY_FS_USE_MNTPOINT context for files */
 	unsigned int behavior;          /* labeling behavior */
 	unsigned char initialized;      /* initialization flag */
+	unsigned char flags;		/* which mount options were specified */
 	unsigned char proc;             /* proc fs */
 	struct mutex lock;
 	struct list_head isec_head;



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Casey Schaufler]

--- Eric Paris <eparis@redhat.com> wrote:

> Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> preparation for NFS to be able to own its own mount options and remove
> the NFS specific code from SELinux.

What is the purpose of security_clone_sb_mnt_opts? I see where it is
being used, but it's not clear to me why it is necessary. The old SELinux
code doesn't clone the options, it filters out the ones it cares about.
If that is the behavior you'd expect of this hook, I suggest the name
reflect that, perhaps security_filter_sb_mnt_opts.


Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Casey Schaufler]

--- Eric Paris <eparis@redhat.com> wrote:

> Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> preparation for NFS to be able to own its own mount options and remove
> the NFS specific code from SELinux.

What is the purpose of security_clone_sb_mnt_opts? I see where it is
being used, but it's not clear to me why it is necessary. The old SELinux
code doesn't clone the options, it filters out the ones it cares about.
If that is the behavior you'd expect of this hook, I suggest the name
reflect that, perhaps security_filter_sb_mnt_opts.


Casey Schaufler
casey@schaufler-ca.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> --- Eric Paris <eparis@redhat.com> wrote:
> 
> > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > preparation for NFS to be able to own its own mount options and remove
> > the NFS specific code from SELinux.
> 
> What is the purpose of security_clone_sb_mnt_opts? I see where it is
> being used, but it's not clear to me why it is necessary. The old SELinux
> code doesn't clone the options, it filters out the ones it cares about.
> If that is the behavior you'd expect of this hook, I suggest the name
> reflect that, perhaps security_filter_sb_mnt_opts.

I'm not sure what you mean by filter and I don't know which code you are
referring to.

At the moment the only user of clone is nfs nohide mounts.  Given an
exports list like

/export *()
/export/nohide *(nohide)

where /export/nohide actually crosses a filesystem boundry the nfs
client will quietly mount the new export on your machine as soon as you
enter that directory.  Assuming the user originally mounted /export with
some security options when nfs mounts this new export there needs to be
some way to propogate those security options forward to this new mount.
So the one usage of clone simply takes the security options from the
superblock related to /export and applies those to the new security
block related to /export/nohide

-Eric


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> --- Eric Paris <eparis@redhat.com> wrote:
> 
> > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > preparation for NFS to be able to own its own mount options and remove
> > the NFS specific code from SELinux.
> 
> What is the purpose of security_clone_sb_mnt_opts? I see where it is
> being used, but it's not clear to me why it is necessary. The old SELinux
> code doesn't clone the options, it filters out the ones it cares about.
> If that is the behavior you'd expect of this hook, I suggest the name
> reflect that, perhaps security_filter_sb_mnt_opts.

I'm not sure what you mean by filter and I don't know which code you are
referring to.

At the moment the only user of clone is nfs nohide mounts.  Given an
exports list like

/export *()
/export/nohide *(nohide)

where /export/nohide actually crosses a filesystem boundry the nfs
client will quietly mount the new export on your machine as soon as you
enter that directory.  Assuming the user originally mounted /export with
some security options when nfs mounts this new export there needs to be
some way to propogate those security options forward to this new mount.
So the one usage of clone simply takes the security options from the
superblock related to /export and applies those to the new security
block related to /export/nohide

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Casey Schaufler]

--- Eric Paris <eparis@redhat.com> wrote:

> On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> > --- Eric Paris <eparis@redhat.com> wrote:
> > 
> > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > preparation for NFS to be able to own its own mount options and remove
> > > the NFS specific code from SELinux.
> > 
> > What is the purpose of security_clone_sb_mnt_opts? I see where it is
> > being used, but it's not clear to me why it is necessary. The old SELinux
> > code doesn't clone the options, it filters out the ones it cares about.
> > If that is the behavior you'd expect of this hook, I suggest the name
> > reflect that, perhaps security_filter_sb_mnt_opts.
> 
> I'm not sure what you mean by filter and I don't know which code you are
> referring to.
> 
> At the moment the only user of clone is nfs nohide mounts.  Given an
> exports list like
> 
> /export *()
> /export/nohide *(nohide)
> 
> where /export/nohide actually crosses a filesystem boundry the nfs
> client will quietly mount the new export on your machine as soon as you
> enter that directory.  Assuming the user originally mounted /export with
> some security options when nfs mounts this new export there needs to be
> some way to propogate those security options forward to this new mount.
> So the one usage of clone simply takes the security options from the
> superblock related to /export and applies those to the new security
> block related to /export/nohide

That is perfectly sensible. Is the intent to clone all the mount options
or just the security mount options? Inquiring LSM writers want to know.


Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Casey Schaufler]

--- Eric Paris <eparis@redhat.com> wrote:

> On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> > --- Eric Paris <eparis@redhat.com> wrote:
> > 
> > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > preparation for NFS to be able to own its own mount options and remove
> > > the NFS specific code from SELinux.
> > 
> > What is the purpose of security_clone_sb_mnt_opts? I see where it is
> > being used, but it's not clear to me why it is necessary. The old SELinux
> > code doesn't clone the options, it filters out the ones it cares about.
> > If that is the behavior you'd expect of this hook, I suggest the name
> > reflect that, perhaps security_filter_sb_mnt_opts.
> 
> I'm not sure what you mean by filter and I don't know which code you are
> referring to.
> 
> At the moment the only user of clone is nfs nohide mounts.  Given an
> exports list like
> 
> /export *()
> /export/nohide *(nohide)
> 
> where /export/nohide actually crosses a filesystem boundry the nfs
> client will quietly mount the new export on your machine as soon as you
> enter that directory.  Assuming the user originally mounted /export with
> some security options when nfs mounts this new export there needs to be
> some way to propogate those security options forward to this new mount.
> So the one usage of clone simply takes the security options from the
> superblock related to /export and applies those to the new security
> block related to /export/nohide

That is perfectly sensible. Is the intent to clone all the mount options
or just the security mount options? Inquiring LSM writers want to know.


Casey Schaufler
casey@schaufler-ca.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 09:28 -0700, Casey Schaufler wrote:
> --- Eric Paris <eparis@redhat.com> wrote:
> 
> > On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> > > --- Eric Paris <eparis@redhat.com> wrote:
> > > 
> > > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > > preparation for NFS to be able to own its own mount options and remove
> > > > the NFS specific code from SELinux.
> > > 
> > > What is the purpose of security_clone_sb_mnt_opts? I see where it is
> > > being used, but it's not clear to me why it is necessary. The old SELinux
> > > code doesn't clone the options, it filters out the ones it cares about.
> > > If that is the behavior you'd expect of this hook, I suggest the name
> > > reflect that, perhaps security_filter_sb_mnt_opts.
> > 
> > I'm not sure what you mean by filter and I don't know which code you are
> > referring to.
> > 
> > At the moment the only user of clone is nfs nohide mounts.  Given an
> > exports list like
> > 
> > /export *()
> > /export/nohide *(nohide)
> > 
> > where /export/nohide actually crosses a filesystem boundry the nfs
> > client will quietly mount the new export on your machine as soon as you
> > enter that directory.  Assuming the user originally mounted /export with
> > some security options when nfs mounts this new export there needs to be
> > some way to propogate those security options forward to this new mount.
> > So the one usage of clone simply takes the security options from the
> > superblock related to /export and applies those to the new security
> > block related to /export/nohide
> 
> That is perfectly sensible. Is the intent to clone all the mount options
> or just the security mount options? Inquiring LSM writers want to know.

I'd have to say the security ones.  It is NFS's problem to make sure
that the options they understand are being handled correctly and I think
it is the security layers problem to make sure the options they
understand are being cloned.  (I tried to make these pretty generic and
extensible for other LSM writers, especially get/set if anyone else
someday makes use of mount options....)

-Eric


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 09:28 -0700, Casey Schaufler wrote:
> --- Eric Paris <eparis@redhat.com> wrote:
> 
> > On Thu, 2007-09-06 at 08:59 -0700, Casey Schaufler wrote:
> > > --- Eric Paris <eparis@redhat.com> wrote:
> > > 
> > > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > > preparation for NFS to be able to own its own mount options and remove
> > > > the NFS specific code from SELinux.
> > > 
> > > What is the purpose of security_clone_sb_mnt_opts? I see where it is
> > > being used, but it's not clear to me why it is necessary. The old SELinux
> > > code doesn't clone the options, it filters out the ones it cares about.
> > > If that is the behavior you'd expect of this hook, I suggest the name
> > > reflect that, perhaps security_filter_sb_mnt_opts.
> > 
> > I'm not sure what you mean by filter and I don't know which code you are
> > referring to.
> > 
> > At the moment the only user of clone is nfs nohide mounts.  Given an
> > exports list like
> > 
> > /export *()
> > /export/nohide *(nohide)
> > 
> > where /export/nohide actually crosses a filesystem boundry the nfs
> > client will quietly mount the new export on your machine as soon as you
> > enter that directory.  Assuming the user originally mounted /export with
> > some security options when nfs mounts this new export there needs to be
> > some way to propogate those security options forward to this new mount.
> > So the one usage of clone simply takes the security options from the
> > superblock related to /export and applies those to the new security
> > block related to /export/nohide
> 
> That is perfectly sensible. Is the intent to clone all the mount options
> or just the security mount options? Inquiring LSM writers want to know.

I'd have to say the security ones.  It is NFS's problem to make sure
that the options they understand are being handled correctly and I think
it is the security layers problem to make sure the options they
understand are being cloned.  (I tried to make these pretty generic and
extensible for other LSM writers, especially get/set if anyone else
someday makes use of mount options....)

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Stephen Smalley]

On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> preparation for NFS to be able to own its own mount options and remove
> the NFS specific code from SELinux.
> 
> >From the point of view of SELinux NFS does not use text based mount
> options so this patch is not something that is going away.  NFS merely
> takes the text options from userspace and puts them into the same binary
> format as always and passes that binary mount data around.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> 
> ---
> 
>  include/linux/security.h          |   70 +++++
>  security/dummy.c                  |   26 ++
>  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
>  security/selinux/include/objsec.h |    1 +
>  4 files changed, 505 insertions(+), 210 deletions(-)

Not a very nice diff to read.

> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1a15526..65f2d91 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -2334,6 +2383,27 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
>  					       struct nameidata *new_nd)
>  { }
>  
> +static inline int security_sb_get_mnt_opts (const struct super_block *sb,
> +					    char ***mount_options,
> +					    int **flags, int *num_opts)
> +{
> +	*mount_options = NULL;
> +	*flags = NULL;
> +	*num_opts = 0;
> +	return 0;
> +}
> +
> +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> +					    char **mount_options,
> +					    int *flags, int num_opts)
> +{
> +	return 0;
> +}

For consistency, this should likely do the same thing as the dummy
function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
going to get called at all unless num_opts is > 0?

> diff --git a/security/dummy.c b/security/dummy.c
> index 853ec22..c3ac361 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -248,6 +248,29 @@ static void dummy_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata
>  	return;
>  }
>  
> +static int dummy_sb_get_mnt_opts(const struct super_block *sb, char ***mount_options,
> +				 int **flags, int *num_opts)
> +{
> +	*mount_options = NULL;
> +	*flags = NULL;
> +	*num_opts = 0;
> +	return 0;
> +}
> +
> +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> +				 int *flags, int num_opts)
> +{
> +	if (unlikely(num_opts))
> +		return -EOPNOTSUPP;
> +	return 0;
> +}

Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
normally get called with no options.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3694662..5ebe30b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
>  	return rc;
>  }
>  
> -static int try_context_mount(struct super_block *sb, void *data)
> +static int sb_finish_set_opts(struct super_block *sb)
>  {
> -	char *context = NULL, *defcontext = NULL;
> -	char *fscontext = NULL, *rootcontext = NULL;
> -	const char *name;
> -	u32 sid;
> -	int alloc = 0, rc = 0, seen = 0;
> -	struct task_security_struct *tsec = current->security;
>  	struct superblock_security_struct *sbsec = sb->s_security;
> +	struct dentry *root = sb->s_root;
> +	struct inode *root_inode = root->d_inode;
> +	int rc = 0;
>  
> -	if (!data)
> -		goto out;
> +	BUG_ON(!ss_initialized);

Why BUG_ON vs. return error?  context mount before initial policy load
is technically possible, even more so if booting permissive and policy
is corrupted.

>  
> -	name = sb->s_type->name;
> +	if (sbsec->initialized)
> +		return 0;

Under what conditions would we reach this point if sbsec was already
initialized?  Should it be an error?  A bug?

> +/*
> + * This function should allow an FS (like NFS) to ask what it's mount security
> + * options were so it can use those later for submounts and the like
> + */
> +static int selinux_get_mnt_opts(const struct super_block *sb,
> +				char ***mount_options, int **mnt_opts_flags,
> +				int *num_opts)
> +{
> +	int rc = 0, i;
> +	struct superblock_security_struct *sbsec = sb->s_security;
> +	char *context = NULL;
> +	u32 len;
> +	/* 
> +	 * if we ever use sbsec flags for anything other than tracking mount
> +	 * settings this is going to need a mask
> +	 */
> +	char tmp = sbsec->flags; 
> +	*num_opts = 0;
> +	*mount_options = NULL;
> +	*mnt_opts_flags = NULL;
> +
> +	for(i = 0; i < 8; i++) {
> +		if (tmp & 0x01)
> +			(*num_opts)++;
> +		tmp >>= 1;
> +	}
>  
> -		while ((p = strsep(&options, "|")) != NULL) {
> -			int token;
> -			substring_t args[MAX_OPT_ARGS];
> +	i = 0;
> +	*mount_options = kcalloc(*num_opts, sizeof(char *), GFP_ATOMIC);
> +	if (!*mount_options) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
>  
> -			if (!*p)
> -				continue;
> +	*mnt_opts_flags = kcalloc(*num_opts, sizeof(int), GFP_ATOMIC);
> +	if (!*mnt_opts_flags) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
>  
> -			token = match_token(p, tokens, args);
> +	if (sbsec->flags & FSCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = FSCONTEXT_MNT;
> +	}
> +	if (sbsec->flags & CONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->mntpoint_sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = CONTEXT_MNT;
> +	}
> +	if (sbsec->flags & DEFCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = DEFCONTEXT_MNT;
> +	}
> +	if (sbsec->flags & ROOTCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);

Isn't that the wrong SID to use?

> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = ROOTCONTEXT_MNT;
> +	}
<snip>
> +/*
> + * Allow filesystems with binary mount data to explicitly set mount point labeling.
> + */
> +int selinux_set_mnt_opts(struct super_block *sb, char **mount_options,
> +				 int *flags, int num_opts)
> +{
> +	int rc = 0, i;
> +	u32 sid;
> +	struct task_security_struct *tsec = current->security;
> +	struct superblock_security_struct *sbsec = sb->s_security;
> +	char *fscontext = NULL, *context = NULL, *rootcontext= NULL;
> +	char *defcontext = NULL;
> +	const char *name = sb->s_type->name;
>  
> -			default:
> -				rc = -EINVAL;
> -				printk(KERN_WARNING "SELinux:  unknown mount "
> -				       "option\n");
> -				goto out_free;
> +	mutex_lock(&sbsec->lock);
>  
> -			}
> -		}
> +	if (sbsec->initialized) {
> +		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
> +			__FUNCTION__);
> +		goto out;
>  	}

BUG_ON?  rc = -EINVAL?  Something other than printk and return 0.

> +static void selinux_sb_clone_mnt_opts (const struct super_block *oldsb,
> +					struct super_block *newsb)
> +{
> +	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
> +	struct superblock_security_struct *newsbsec = newsb->s_security;
> +
> +	int set_fscontext =	(oldsbsec->flags & FSCONTEXT_MNT);
> +	int set_context =	(oldsbsec->flags & CONTEXT_MNT);
> +	int set_rootcontext =	(oldsbsec->flags & ROOTCONTEXT_MNT);
> +
> +	mutex_lock(&newsbsec->lock);
> +
> +	newsbsec->flags = oldsbsec->flags;
> +
> +	newsbsec->sid = oldsbsec->sid;
> +	newsbsec->def_sid = oldsbsec->def_sid;
> +	newsbsec->behavior = oldsbsec->behavior;
> +
> +	if (set_context) {
> +		u32 sid = oldsbsec->mntpoint_sid;
> +
> +		if (!set_fscontext)
> +			newsbsec->sid = sid;
> +		if (!set_rootcontext) {
> +			struct inode *newinode = newsb->s_root->d_inode;
> +			struct inode_security_struct *newisec = newinode->i_security;
> +			newisec->sid = sid;
> +		}
> +		newsbsec->mntpoint_sid = sid;
> +	}
> +	if (set_rootcontext) {
> +		const struct inode *oldinode = oldsb->s_root->d_inode;
> +		const struct inode_security_struct *oldisec = oldinode->i_security;
> +		struct inode *newinode = newsb->s_root->d_inode;
> +		struct inode_security_struct *newisec = newinode->i_security;
> +
> +		newisec->sid = oldisec->sid;
>  	}
> +
> +	sb_finish_set_opts(newsb);
> +	mutex_unlock(&newsbsec->lock);
> +}
> +
> +/*
> + * string mount options parsing and call to function to set the sbsec
> + */
> +static int try_context_mount(struct super_block *sb, void *data)
> +{
> +	char *context = NULL, *defcontext = NULL;
> +	char *fscontext = NULL, *rootcontext = NULL;
> +	int rc = 0;
> +	char *p, *options = data;
> +	/* selinux only know about 4 mount options */
> +	char *mnt_opts[4];
> +	int mnt_opts_flags[4], num_mnt_opts = 0;

Can we #define that once and use it?

> +	int con_from_nfs = 0;
> +
> +	if (!data)
> +		goto out;
> +
> +	/* with the nfs patch this will become a goto out; */

Hmm...not sure this is worth separating out for the second patch,
although it would mean that a kernel with only the first patch applied
won't have nfs context mount support at all.

> +	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
> +		const char *name = sb->s_type->name;
> +		/* NFS we understand. */
> +		if (!strcmp(name, "nfs")) {
> +			struct nfs_mount_data *d = data;
> +
> +			if (d->version !=  NFS_MOUNT_VERSION)
> +				goto out;
> +
> +			if (d->context[0])
> +				context = d->context;
> +			con_from_nfs = 1;
> +		} else
> +			goto out;
> +	} else {
> +
> +	/* Standard string-based options. */
> +	while ((p = strsep(&options, "|")) != NULL) {
> +		int token;
> +		substring_t args[MAX_OPT_ARGS];
> +
> +		if (!*p)
> +			continue;
> +
> +		token = match_token(p, tokens, args);
> +
> +		switch (token) {
> +		case Opt_context:
> +			if (context || defcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			context = match_strdup(&args[0]);
> +			if (!context) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_fscontext:
> +			if (fscontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			fscontext = match_strdup(&args[0]);
> +			if (!fscontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_rootcontext:
> +			if (rootcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			rootcontext = match_strdup(&args[0]);
> +			if (!rootcontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_defcontext:
> +			if (context || defcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			defcontext = match_strdup(&args[0]);
> +			if (!defcontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		default:
> +			rc = -EINVAL;
> +			printk(KERN_WARNING "SELinux:  unknown mount option\n");
> +			goto out_err;
> +
> +		}
> +	}
> +	} // else for binary mount data.  delete with nfs patch

Especially given this kind of ugliness.

> +
> +	if(fscontext){

Coding style nit throughout - if (x) { not if(x){

> +		mnt_opts[num_mnt_opts] = fscontext;
> +		mnt_opts_flags[num_mnt_opts++] = FSCONTEXT_MNT;
> +	}
> +	if(context){
> +		mnt_opts[num_mnt_opts] = context;
> +		mnt_opts_flags[num_mnt_opts++] = CONTEXT_MNT;
> +	}
> +	if(rootcontext){
> +		mnt_opts[num_mnt_opts] = rootcontext;
> +		mnt_opts_flags[num_mnt_opts++] = ROOTCONTEXT_MNT;
> +	}
> +	if(defcontext){
> +		mnt_opts[num_mnt_opts] = defcontext;
> +		mnt_opts_flags[num_mnt_opts++] = DEFCONTEXT_MNT;
> +	}
> +
>  out:
> +	rc = selinux_set_mnt_opts(sb, mnt_opts, mnt_opts_flags, num_mnt_opts);
> +out_err:
> +	/* remove conditional when we remove nfs */
> +	if (!con_from_nfs)
> +		kfree(context);

I'd just kill it now in this patch.

> +	kfree(defcontext);
> +	kfree(fscontext);
> +	kfree(rootcontext);
>  	return rc;
>  }
>  
> +/* 
> + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> + * set the same way in another thread and fs_use, hmmmmmm 
> + */

Not sure I follow; you do take the lock still in set_mnts_opts before
doing the above AFAICS.  sbsec->list though is another matter.

>  static int superblock_doinit(struct super_block *sb, void *data)
>  {
>  	struct superblock_security_struct *sbsec = sb->s_security;
> -	struct dentry *root = sb->s_root;
> -	struct inode *inode = root->d_inode;
>  	int rc = 0;
>  
> -	mutex_lock(&sbsec->lock);
>  	if (sbsec->initialized)
>  		goto out;

One of the things that needs to be fixed is handling of a context mount
option when the superblock has previously been setup.  At present (and
still true of your patch), if one mounts /home/eparis with one context
mount and then tries to mount /home/sds with a different context mount,
the latter proceeds with no indication that the context mount option was
ignored (because we immediately return with success if already
initialized, and they'll end up with the same superblock if they are
from the same filesystem).  So we need to at least check to see if data
is set and return an error if sbsec is already initialized so that
userspace gets an indication of the fact.

-- 
Stephen Smalley
National Security Agency


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Stephen Smalley]

On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> preparation for NFS to be able to own its own mount options and remove
> the NFS specific code from SELinux.
> 
> >From the point of view of SELinux NFS does not use text based mount
> options so this patch is not something that is going away.  NFS merely
> takes the text options from userspace and puts them into the same binary
> format as always and passes that binary mount data around.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> 
> ---
> 
>  include/linux/security.h          |   70 +++++
>  security/dummy.c                  |   26 ++
>  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
>  security/selinux/include/objsec.h |    1 +
>  4 files changed, 505 insertions(+), 210 deletions(-)

Not a very nice diff to read.

> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1a15526..65f2d91 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -2334,6 +2383,27 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
>  					       struct nameidata *new_nd)
>  { }
>  
> +static inline int security_sb_get_mnt_opts (const struct super_block *sb,
> +					    char ***mount_options,
> +					    int **flags, int *num_opts)
> +{
> +	*mount_options = NULL;
> +	*flags = NULL;
> +	*num_opts = 0;
> +	return 0;
> +}
> +
> +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> +					    char **mount_options,
> +					    int *flags, int num_opts)
> +{
> +	return 0;
> +}

For consistency, this should likely do the same thing as the dummy
function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
going to get called at all unless num_opts is > 0?

> diff --git a/security/dummy.c b/security/dummy.c
> index 853ec22..c3ac361 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -248,6 +248,29 @@ static void dummy_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata
>  	return;
>  }
>  
> +static int dummy_sb_get_mnt_opts(const struct super_block *sb, char ***mount_options,
> +				 int **flags, int *num_opts)
> +{
> +	*mount_options = NULL;
> +	*flags = NULL;
> +	*num_opts = 0;
> +	return 0;
> +}
> +
> +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> +				 int *flags, int num_opts)
> +{
> +	if (unlikely(num_opts))
> +		return -EOPNOTSUPP;
> +	return 0;
> +}

Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
normally get called with no options.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3694662..5ebe30b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
>  	return rc;
>  }
>  
> -static int try_context_mount(struct super_block *sb, void *data)
> +static int sb_finish_set_opts(struct super_block *sb)
>  {
> -	char *context = NULL, *defcontext = NULL;
> -	char *fscontext = NULL, *rootcontext = NULL;
> -	const char *name;
> -	u32 sid;
> -	int alloc = 0, rc = 0, seen = 0;
> -	struct task_security_struct *tsec = current->security;
>  	struct superblock_security_struct *sbsec = sb->s_security;
> +	struct dentry *root = sb->s_root;
> +	struct inode *root_inode = root->d_inode;
> +	int rc = 0;
>  
> -	if (!data)
> -		goto out;
> +	BUG_ON(!ss_initialized);

Why BUG_ON vs. return error?  context mount before initial policy load
is technically possible, even more so if booting permissive and policy
is corrupted.

>  
> -	name = sb->s_type->name;
> +	if (sbsec->initialized)
> +		return 0;

Under what conditions would we reach this point if sbsec was already
initialized?  Should it be an error?  A bug?

> +/*
> + * This function should allow an FS (like NFS) to ask what it's mount security
> + * options were so it can use those later for submounts and the like
> + */
> +static int selinux_get_mnt_opts(const struct super_block *sb,
> +				char ***mount_options, int **mnt_opts_flags,
> +				int *num_opts)
> +{
> +	int rc = 0, i;
> +	struct superblock_security_struct *sbsec = sb->s_security;
> +	char *context = NULL;
> +	u32 len;
> +	/* 
> +	 * if we ever use sbsec flags for anything other than tracking mount
> +	 * settings this is going to need a mask
> +	 */
> +	char tmp = sbsec->flags; 
> +	*num_opts = 0;
> +	*mount_options = NULL;
> +	*mnt_opts_flags = NULL;
> +
> +	for(i = 0; i < 8; i++) {
> +		if (tmp & 0x01)
> +			(*num_opts)++;
> +		tmp >>= 1;
> +	}
>  
> -		while ((p = strsep(&options, "|")) != NULL) {
> -			int token;
> -			substring_t args[MAX_OPT_ARGS];
> +	i = 0;
> +	*mount_options = kcalloc(*num_opts, sizeof(char *), GFP_ATOMIC);
> +	if (!*mount_options) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
>  
> -			if (!*p)
> -				continue;
> +	*mnt_opts_flags = kcalloc(*num_opts, sizeof(int), GFP_ATOMIC);
> +	if (!*mnt_opts_flags) {
> +		rc = -ENOMEM;
> +		goto out_free;
> +	}
>  
> -			token = match_token(p, tokens, args);
> +	if (sbsec->flags & FSCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = FSCONTEXT_MNT;
> +	}
> +	if (sbsec->flags & CONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->mntpoint_sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = CONTEXT_MNT;
> +	}
> +	if (sbsec->flags & DEFCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = DEFCONTEXT_MNT;
> +	}
> +	if (sbsec->flags & ROOTCONTEXT_MNT) {
> +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);

Isn't that the wrong SID to use?

> +		if (rc)
> +			goto out_free;
> +		(*mount_options)[i] = context;
> +		(*mnt_opts_flags)[i++] = ROOTCONTEXT_MNT;
> +	}
<snip>
> +/*
> + * Allow filesystems with binary mount data to explicitly set mount point labeling.
> + */
> +int selinux_set_mnt_opts(struct super_block *sb, char **mount_options,
> +				 int *flags, int num_opts)
> +{
> +	int rc = 0, i;
> +	u32 sid;
> +	struct task_security_struct *tsec = current->security;
> +	struct superblock_security_struct *sbsec = sb->s_security;
> +	char *fscontext = NULL, *context = NULL, *rootcontext= NULL;
> +	char *defcontext = NULL;
> +	const char *name = sb->s_type->name;
>  
> -			default:
> -				rc = -EINVAL;
> -				printk(KERN_WARNING "SELinux:  unknown mount "
> -				       "option\n");
> -				goto out_free;
> +	mutex_lock(&sbsec->lock);
>  
> -			}
> -		}
> +	if (sbsec->initialized) {
> +		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
> +			__FUNCTION__);
> +		goto out;
>  	}

BUG_ON?  rc = -EINVAL?  Something other than printk and return 0.

> +static void selinux_sb_clone_mnt_opts (const struct super_block *oldsb,
> +					struct super_block *newsb)
> +{
> +	const struct superblock_security_struct *oldsbsec = oldsb->s_security;
> +	struct superblock_security_struct *newsbsec = newsb->s_security;
> +
> +	int set_fscontext =	(oldsbsec->flags & FSCONTEXT_MNT);
> +	int set_context =	(oldsbsec->flags & CONTEXT_MNT);
> +	int set_rootcontext =	(oldsbsec->flags & ROOTCONTEXT_MNT);
> +
> +	mutex_lock(&newsbsec->lock);
> +
> +	newsbsec->flags = oldsbsec->flags;
> +
> +	newsbsec->sid = oldsbsec->sid;
> +	newsbsec->def_sid = oldsbsec->def_sid;
> +	newsbsec->behavior = oldsbsec->behavior;
> +
> +	if (set_context) {
> +		u32 sid = oldsbsec->mntpoint_sid;
> +
> +		if (!set_fscontext)
> +			newsbsec->sid = sid;
> +		if (!set_rootcontext) {
> +			struct inode *newinode = newsb->s_root->d_inode;
> +			struct inode_security_struct *newisec = newinode->i_security;
> +			newisec->sid = sid;
> +		}
> +		newsbsec->mntpoint_sid = sid;
> +	}
> +	if (set_rootcontext) {
> +		const struct inode *oldinode = oldsb->s_root->d_inode;
> +		const struct inode_security_struct *oldisec = oldinode->i_security;
> +		struct inode *newinode = newsb->s_root->d_inode;
> +		struct inode_security_struct *newisec = newinode->i_security;
> +
> +		newisec->sid = oldisec->sid;
>  	}
> +
> +	sb_finish_set_opts(newsb);
> +	mutex_unlock(&newsbsec->lock);
> +}
> +
> +/*
> + * string mount options parsing and call to function to set the sbsec
> + */
> +static int try_context_mount(struct super_block *sb, void *data)
> +{
> +	char *context = NULL, *defcontext = NULL;
> +	char *fscontext = NULL, *rootcontext = NULL;
> +	int rc = 0;
> +	char *p, *options = data;
> +	/* selinux only know about 4 mount options */
> +	char *mnt_opts[4];
> +	int mnt_opts_flags[4], num_mnt_opts = 0;

Can we #define that once and use it?

> +	int con_from_nfs = 0;
> +
> +	if (!data)
> +		goto out;
> +
> +	/* with the nfs patch this will become a goto out; */

Hmm...not sure this is worth separating out for the second patch,
although it would mean that a kernel with only the first patch applied
won't have nfs context mount support at all.

> +	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
> +		const char *name = sb->s_type->name;
> +		/* NFS we understand. */
> +		if (!strcmp(name, "nfs")) {
> +			struct nfs_mount_data *d = data;
> +
> +			if (d->version !=  NFS_MOUNT_VERSION)
> +				goto out;
> +
> +			if (d->context[0])
> +				context = d->context;
> +			con_from_nfs = 1;
> +		} else
> +			goto out;
> +	} else {
> +
> +	/* Standard string-based options. */
> +	while ((p = strsep(&options, "|")) != NULL) {
> +		int token;
> +		substring_t args[MAX_OPT_ARGS];
> +
> +		if (!*p)
> +			continue;
> +
> +		token = match_token(p, tokens, args);
> +
> +		switch (token) {
> +		case Opt_context:
> +			if (context || defcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			context = match_strdup(&args[0]);
> +			if (!context) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_fscontext:
> +			if (fscontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			fscontext = match_strdup(&args[0]);
> +			if (!fscontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_rootcontext:
> +			if (rootcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			rootcontext = match_strdup(&args[0]);
> +			if (!rootcontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		case Opt_defcontext:
> +			if (context || defcontext) {
> +				rc = -EINVAL;
> +				printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
> +				goto out_err;
> +			}
> +			defcontext = match_strdup(&args[0]);
> +			if (!defcontext) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +			break;
> +
> +		default:
> +			rc = -EINVAL;
> +			printk(KERN_WARNING "SELinux:  unknown mount option\n");
> +			goto out_err;
> +
> +		}
> +	}
> +	} // else for binary mount data.  delete with nfs patch

Especially given this kind of ugliness.

> +
> +	if(fscontext){

Coding style nit throughout - if (x) { not if(x){

> +		mnt_opts[num_mnt_opts] = fscontext;
> +		mnt_opts_flags[num_mnt_opts++] = FSCONTEXT_MNT;
> +	}
> +	if(context){
> +		mnt_opts[num_mnt_opts] = context;
> +		mnt_opts_flags[num_mnt_opts++] = CONTEXT_MNT;
> +	}
> +	if(rootcontext){
> +		mnt_opts[num_mnt_opts] = rootcontext;
> +		mnt_opts_flags[num_mnt_opts++] = ROOTCONTEXT_MNT;
> +	}
> +	if(defcontext){
> +		mnt_opts[num_mnt_opts] = defcontext;
> +		mnt_opts_flags[num_mnt_opts++] = DEFCONTEXT_MNT;
> +	}
> +
>  out:
> +	rc = selinux_set_mnt_opts(sb, mnt_opts, mnt_opts_flags, num_mnt_opts);
> +out_err:
> +	/* remove conditional when we remove nfs */
> +	if (!con_from_nfs)
> +		kfree(context);

I'd just kill it now in this patch.

> +	kfree(defcontext);
> +	kfree(fscontext);
> +	kfree(rootcontext);
>  	return rc;
>  }
>  
> +/* 
> + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> + * set the same way in another thread and fs_use, hmmmmmm 
> + */

Not sure I follow; you do take the lock still in set_mnts_opts before
doing the above AFAICS.  sbsec->list though is another matter.

>  static int superblock_doinit(struct super_block *sb, void *data)
>  {
>  	struct superblock_security_struct *sbsec = sb->s_security;
> -	struct dentry *root = sb->s_root;
> -	struct inode *inode = root->d_inode;
>  	int rc = 0;
>  
> -	mutex_lock(&sbsec->lock);
>  	if (sbsec->initialized)
>  		goto out;

One of the things that needs to be fixed is handling of a context mount
option when the superblock has previously been setup.  At present (and
still true of your patch), if one mounts /home/eparis with one context
mount and then tries to mount /home/sds with a different context mount,
the latter proceeds with no indication that the context mount option was
ignored (because we immediately return with success if already
initialized, and they'll end up with the same superblock if they are
from the same filesystem).  So we need to at least check to see if data
is set and return an error if sbsec is already initialized so that
userspace gets an indication of the fact.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 14:33 -0400, Stephen Smalley wrote:
> On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > preparation for NFS to be able to own its own mount options and remove
> > the NFS specific code from SELinux.
> > 
> > >From the point of view of SELinux NFS does not use text based mount
> > options so this patch is not something that is going away.  NFS merely
> > takes the text options from userspace and puts them into the same binary
> > format as always and passes that binary mount data around.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > 
> > ---
> > 
> >  include/linux/security.h          |   70 +++++
> >  security/dummy.c                  |   26 ++
> >  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
> >  security/selinux/include/objsec.h |    1 +
> >  4 files changed, 505 insertions(+), 210 deletions(-)
> 
> Not a very nice diff to read.

I couldn't think of a great way to make it smaller.  I certainly
understand....

> 
> > diff --git a/include/linux/security.h b/include/linux/security.h

> > +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> > +					    char **mount_options,
> > +					    int *flags, int num_opts)
> > +{
> > +	return 0;
> > +}
> 
> For consistency, this should likely do the same thing as the dummy
> function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
> going to get called at all unless num_opts is > 0?

EOPNOTSUPP seems like the right thing.  selinux=0 and mount nfs with
context=

> 
> > diff --git a/security/dummy.c b/security/dummy.c

> > +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> > +				 int *flags, int num_opts)
> > +{
> > +	if (unlikely(num_opts))
> > +		return -EOPNOTSUPP;
> > +	return 0;
> > +}
> 
> Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
> normally get called with no options.

I'd rather not rely on the FS to have to follow this convention.  Its
written so that num_opts=0 will work fine.
> 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 3694662..5ebe30b 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
> >  	return rc;
> >  }
> >  
> > -static int try_context_mount(struct super_block *sb, void *data)
> > +static int sb_finish_set_opts(struct super_block *sb)
> >  {
> > -	char *context = NULL, *defcontext = NULL;
> > -	char *fscontext = NULL, *rootcontext = NULL;
> > -	const char *name;
> > -	u32 sid;
> > -	int alloc = 0, rc = 0, seen = 0;
> > -	struct task_security_struct *tsec = current->security;
> >  	struct superblock_security_struct *sbsec = sb->s_security;
> > +	struct dentry *root = sb->s_root;
> > +	struct inode *root_inode = root->d_inode;
> > +	int rc = 0;
> >  
> > -	if (!data)
> > -		goto out;
> > +	BUG_ON(!ss_initialized);
> 
> Why BUG_ON vs. return error?  context mount before initial policy load
> is technically possible, even more so if booting permissive and policy
> is corrupted.

How should your example case be handled?  we can't store that context
information for later.  The only way we get here is through a direct
caller to set or clone from the FS since normally we would come through
superblock_doinit->try_context_mount->set_sb...->here and ss_initialized
would already have been taken care of.  Should I just dump the
superblock on the list to deal with later and not worry that I was given
mount options (doesn't seem like we have a good solution to your problem
now)?

I like an error code better, what fits?  It should be dealt with in
selinux_set_mnt_opts/sb_doinit and not here.  But still the question
remains.

> 
> >  
> > -	name = sb->s_type->name;
> > +	if (sbsec->initialized)
> > +		return 0;
> 
> Under what conditions would we reach this point if sbsec was already
> initialized?  Should it be an error?  A bug?

We can't.  it is caught in both superblock_doinit and
selinux_set_sb_mnt_opts.


> > +	}
> > +	if (sbsec->flags & ROOTCONTEXT_MNT) {
> > +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
> 
> Isn't that the wrong SID to use?
yes.  /me is quite embarrassed about that one...


> > +	if (sbsec->initialized) {
> > +		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
> > +			__FUNCTION__);
> > +		goto out;
> >  	}
> 
> BUG_ON?  rc = -EINVAL?  Something other than printk and return 0.
This is actually where we need to point out the mount the same thing
with different options which you talk about at the bottom.  I'll check
this closely and make it a better message.

> > +static int try_context_mount(struct super_block *sb, void *data)
> > +{
> > +	char *context = NULL, *defcontext = NULL;
> > +	char *fscontext = NULL, *rootcontext = NULL;
> > +	int rc = 0;
> > +	char *p, *options = data;
> > +	/* selinux only know about 4 mount options */
> > +	char *mnt_opts[4];
> > +	int mnt_opts_flags[4], num_mnt_opts = 0;
> 
> Can we #define that once and use it?

the variable declarations?  I guess I could.  They aren't used in many
other places, but having [4] randomly in the code is probably not the
best idea.

> 
> > +	int con_from_nfs = 0;
> > +
> > +	if (!data)
> > +		goto out;
> > +
> > +	/* with the nfs patch this will become a goto out; */
> 
> Hmm...not sure this is worth separating out for the second patch,
> although it would mean that a kernel with only the first patch applied
> won't have nfs context mount support at all.

And it's just a couple lines.
> 
> > +	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
> > +		const char *name = sb->s_type->name;

> > +
> > +		}
> > +	}
> > +	} // else for binary mount data.  delete with nfs patch
> 
> Especially given this kind of ugliness.

I liked it as well because the NFS specific patch clearly shows the NFS
people that I pulled the ugly hack out.
> 
> > +
> > +	if(fscontext){
> 
> Coding style nit throughout - if (x) { not if(x){
stupid lazy thumbs.

> >  
> > +/* 
> > + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> > + * set the same way in another thread and fs_use, hmmmmmm 
> > + */
> 
> Not sure I follow; you do take the lock still in set_mnts_opts before
> doing the above AFAICS.  sbsec->list though is another matter.

comment doesn't belong.  I wasn't taking it and was still setting ->proc
here but realized it was a stupid idea.  Never removed the comment.

> 
> >  static int superblock_doinit(struct super_block *sb, void *data)
> >  {
> >  	struct superblock_security_struct *sbsec = sb->s_security;
> > -	struct dentry *root = sb->s_root;
> > -	struct inode *inode = root->d_inode;
> >  	int rc = 0;
> >  
> > -	mutex_lock(&sbsec->lock);
> >  	if (sbsec->initialized)
> >  		goto out;
> 
> One of the things that needs to be fixed is handling of a context mount
> option when the superblock has previously been setup.  At present (and
> still true of your patch), if one mounts /home/eparis with one context
> mount and then tries to mount /home/sds with a different context mount,
> the latter proceeds with no indication that the context mount option was
> ignored (because we immediately return with success if already
> initialized, and they'll end up with the same superblock if they are
> from the same filesystem).  So we need to at least check to see if data
> is set and return an error if sbsec is already initialized so that
> userspace gets an indication of the fact.

I pointed out where I think we need to do this, but what kind of error
do we want to return?  Do we actually want the mount command to fail?


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Eric Paris]

On Thu, 2007-09-06 at 14:33 -0400, Stephen Smalley wrote:
> On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > preparation for NFS to be able to own its own mount options and remove
> > the NFS specific code from SELinux.
> > 
> > >From the point of view of SELinux NFS does not use text based mount
> > options so this patch is not something that is going away.  NFS merely
> > takes the text options from userspace and puts them into the same binary
> > format as always and passes that binary mount data around.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > 
> > ---
> > 
> >  include/linux/security.h          |   70 +++++
> >  security/dummy.c                  |   26 ++
> >  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
> >  security/selinux/include/objsec.h |    1 +
> >  4 files changed, 505 insertions(+), 210 deletions(-)
> 
> Not a very nice diff to read.

I couldn't think of a great way to make it smaller.  I certainly
understand....

> 
> > diff --git a/include/linux/security.h b/include/linux/security.h

> > +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> > +					    char **mount_options,
> > +					    int *flags, int num_opts)
> > +{
> > +	return 0;
> > +}
> 
> For consistency, this should likely do the same thing as the dummy
> function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
> going to get called at all unless num_opts is > 0?

EOPNOTSUPP seems like the right thing.  selinux=0 and mount nfs with
context=

> 
> > diff --git a/security/dummy.c b/security/dummy.c

> > +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> > +				 int *flags, int num_opts)
> > +{
> > +	if (unlikely(num_opts))
> > +		return -EOPNOTSUPP;
> > +	return 0;
> > +}
> 
> Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
> normally get called with no options.

I'd rather not rely on the FS to have to follow this convention.  Its
written so that num_opts=0 will work fine.
> 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 3694662..5ebe30b 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
> >  	return rc;
> >  }
> >  
> > -static int try_context_mount(struct super_block *sb, void *data)
> > +static int sb_finish_set_opts(struct super_block *sb)
> >  {
> > -	char *context = NULL, *defcontext = NULL;
> > -	char *fscontext = NULL, *rootcontext = NULL;
> > -	const char *name;
> > -	u32 sid;
> > -	int alloc = 0, rc = 0, seen = 0;
> > -	struct task_security_struct *tsec = current->security;
> >  	struct superblock_security_struct *sbsec = sb->s_security;
> > +	struct dentry *root = sb->s_root;
> > +	struct inode *root_inode = root->d_inode;
> > +	int rc = 0;
> >  
> > -	if (!data)
> > -		goto out;
> > +	BUG_ON(!ss_initialized);
> 
> Why BUG_ON vs. return error?  context mount before initial policy load
> is technically possible, even more so if booting permissive and policy
> is corrupted.

How should your example case be handled?  we can't store that context
information for later.  The only way we get here is through a direct
caller to set or clone from the FS since normally we would come through
superblock_doinit->try_context_mount->set_sb...->here and ss_initialized
would already have been taken care of.  Should I just dump the
superblock on the list to deal with later and not worry that I was given
mount options (doesn't seem like we have a good solution to your problem
now)?

I like an error code better, what fits?  It should be dealt with in
selinux_set_mnt_opts/sb_doinit and not here.  But still the question
remains.

> 
> >  
> > -	name = sb->s_type->name;
> > +	if (sbsec->initialized)
> > +		return 0;
> 
> Under what conditions would we reach this point if sbsec was already
> initialized?  Should it be an error?  A bug?

We can't.  it is caught in both superblock_doinit and
selinux_set_sb_mnt_opts.


> > +	}
> > +	if (sbsec->flags & ROOTCONTEXT_MNT) {
> > +		rc = security_sid_to_context(sbsec->def_sid, &context, &len);
> 
> Isn't that the wrong SID to use?
yes.  /me is quite embarrassed about that one...


> > +	if (sbsec->initialized) {
> > +		printk(KERN_WARNING "%s: we are here with sbsec->initialized == 1\n",
> > +			__FUNCTION__);
> > +		goto out;
> >  	}
> 
> BUG_ON?  rc = -EINVAL?  Something other than printk and return 0.
This is actually where we need to point out the mount the same thing
with different options which you talk about at the bottom.  I'll check
this closely and make it a better message.

> > +static int try_context_mount(struct super_block *sb, void *data)
> > +{
> > +	char *context = NULL, *defcontext = NULL;
> > +	char *fscontext = NULL, *rootcontext = NULL;
> > +	int rc = 0;
> > +	char *p, *options = data;
> > +	/* selinux only know about 4 mount options */
> > +	char *mnt_opts[4];
> > +	int mnt_opts_flags[4], num_mnt_opts = 0;
> 
> Can we #define that once and use it?

the variable declarations?  I guess I could.  They aren't used in many
other places, but having [4] randomly in the code is probably not the
best idea.

> 
> > +	int con_from_nfs = 0;
> > +
> > +	if (!data)
> > +		goto out;
> > +
> > +	/* with the nfs patch this will become a goto out; */
> 
> Hmm...not sure this is worth separating out for the second patch,
> although it would mean that a kernel with only the first patch applied
> won't have nfs context mount support at all.

And it's just a couple lines.
> 
> > +	if (sb->s_type->fs_flags & FS_BINARY_MOUNTDATA) {
> > +		const char *name = sb->s_type->name;

> > +
> > +		}
> > +	}
> > +	} // else for binary mount data.  delete with nfs patch
> 
> Especially given this kind of ugliness.

I liked it as well because the NFS specific patch clearly shows the NFS
people that I pulled the ugly hack out.
> 
> > +
> > +	if(fscontext){
> 
> Coding style nit throughout - if (x) { not if(x){
stupid lazy thumbs.

> >  
> > +/* 
> > + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> > + * set the same way in another thread and fs_use, hmmmmmm 
> > + */
> 
> Not sure I follow; you do take the lock still in set_mnts_opts before
> doing the above AFAICS.  sbsec->list though is another matter.

comment doesn't belong.  I wasn't taking it and was still setting ->proc
here but realized it was a stupid idea.  Never removed the comment.

> 
> >  static int superblock_doinit(struct super_block *sb, void *data)
> >  {
> >  	struct superblock_security_struct *sbsec = sb->s_security;
> > -	struct dentry *root = sb->s_root;
> > -	struct inode *inode = root->d_inode;
> >  	int rc = 0;
> >  
> > -	mutex_lock(&sbsec->lock);
> >  	if (sbsec->initialized)
> >  		goto out;
> 
> One of the things that needs to be fixed is handling of a context mount
> option when the superblock has previously been setup.  At present (and
> still true of your patch), if one mounts /home/eparis with one context
> mount and then tries to mount /home/sds with a different context mount,
> the latter proceeds with no indication that the context mount option was
> ignored (because we immediately return with success if already
> initialized, and they'll end up with the same superblock if they are
> from the same filesystem).  So we need to at least check to see if data
> is set and return an error if sbsec is already initialized so that
> userspace gets an indication of the fact.

I pointed out where I think we need to do this, but what kind of error
do we want to return?  Do we actually want the mount command to fail?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Stephen Smalley]

On Thu, 2007-09-06 at 15:21 -0400, Eric Paris wrote:
> On Thu, 2007-09-06 at 14:33 -0400, Stephen Smalley wrote:
> > On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > preparation for NFS to be able to own its own mount options and remove
> > > the NFS specific code from SELinux.
> > > 
> > > >From the point of view of SELinux NFS does not use text based mount
> > > options so this patch is not something that is going away.  NFS merely
> > > takes the text options from userspace and puts them into the same binary
> > > format as always and passes that binary mount data around.
> > > 
> > > Signed-off-by: Eric Paris <eparis@redhat.com>
> > > 
> > > ---
> > > 
> > >  include/linux/security.h          |   70 +++++
> > >  security/dummy.c                  |   26 ++
> > >  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
> > >  security/selinux/include/objsec.h |    1 +
> > >  4 files changed, 505 insertions(+), 210 deletions(-)
> > 
> > Not a very nice diff to read.
> 
> I couldn't think of a great way to make it smaller.  I certainly
> understand....
> 
> > 
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> 
> > > +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> > > +					    char **mount_options,
> > > +					    int *flags, int num_opts)
> > > +{
> > > +	return 0;
> > > +}
> > 
> > For consistency, this should likely do the same thing as the dummy
> > function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
> > going to get called at all unless num_opts is > 0?
> 
> EOPNOTSUPP seems like the right thing.  selinux=0 and mount nfs with
> context=
> 
> > 
> > > diff --git a/security/dummy.c b/security/dummy.c
> 
> > > +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> > > +				 int *flags, int num_opts)
> > > +{
> > > +	if (unlikely(num_opts))
> > > +		return -EOPNOTSUPP;
> > > +	return 0;
> > > +}
> > 
> > Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
> > normally get called with no options.
> 
> I'd rather not rely on the FS to have to follow this convention.  Its
> written so that num_opts=0 will work fine.
> > 
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 3694662..5ebe30b 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
> > >  	return rc;
> > >  }
> > >  
> > > -static int try_context_mount(struct super_block *sb, void *data)
> > > +static int sb_finish_set_opts(struct super_block *sb)
> > >  {
> > > -	char *context = NULL, *defcontext = NULL;
> > > -	char *fscontext = NULL, *rootcontext = NULL;
> > > -	const char *name;
> > > -	u32 sid;
> > > -	int alloc = 0, rc = 0, seen = 0;
> > > -	struct task_security_struct *tsec = current->security;
> > >  	struct superblock_security_struct *sbsec = sb->s_security;
> > > +	struct dentry *root = sb->s_root;
> > > +	struct inode *root_inode = root->d_inode;
> > > +	int rc = 0;
> > >  
> > > -	if (!data)
> > > -		goto out;
> > > +	BUG_ON(!ss_initialized);
> > 
> > Why BUG_ON vs. return error?  context mount before initial policy load
> > is technically possible, even more so if booting permissive and policy
> > is corrupted.
> 
> How should your example case be handled?  we can't store that context
> information for later.

Error code.

>   The only way we get here is through a direct
> caller to set or clone from the FS since normally we would come through
> superblock_doinit->try_context_mount->set_sb...->here and ss_initialized
> would already have been taken care of.  Should I just dump the
> superblock on the list to deal with later and not worry that I was given
> mount options (doesn't seem like we have a good solution to your problem
> now)?
> 
> I like an error code better, what fits?  It should be dealt with in
> selinux_set_mnt_opts/sb_doinit and not here.  But still the question
> remains.

-EOPNOTSUPP or -EINVAL.  Possibly with a printk.

> > 
> > >  
> > > -	name = sb->s_type->name;
> > > +	if (sbsec->initialized)
> > > +		return 0;
> > 
> > Under what conditions would we reach this point if sbsec was already
> > initialized?  Should it be an error?  A bug?
> 
> We can't.  it is caught in both superblock_doinit and
> selinux_set_sb_mnt_opts.

So BUG_ON?  Or drop it.

> > > +static int try_context_mount(struct super_block *sb, void *data)
> > > +{
> > > +	char *context = NULL, *defcontext = NULL;
> > > +	char *fscontext = NULL, *rootcontext = NULL;
> > > +	int rc = 0;
> > > +	char *p, *options = data;
> > > +	/* selinux only know about 4 mount options */
> > > +	char *mnt_opts[4];
> > > +	int mnt_opts_flags[4], num_mnt_opts = 0;
> > 
> > Can we #define that once and use it?
> 
> the variable declarations?  I guess I could.  They aren't used in many
> other places, but having [4] randomly in the code is probably not the
> best idea.

Yes, I just meant replacing 4 with a #define'd symbol.

> > >  
> > > +/* 
> > > + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> > > + * set the same way in another thread and fs_use, hmmmmmm 
> > > + */
> > 
> > Not sure I follow; you do take the lock still in set_mnts_opts before
> > doing the above AFAICS.  sbsec->list though is another matter.
> 
> comment doesn't belong.  I wasn't taking it and was still setting ->proc
> here but realized it was a stupid idea.  Never removed the comment.

Ok, but sbsec->list might still be an issue technically even if not in
practice.  There is a global spinlock on the whole list, but you could
end up adding the superblock twice if you don't take the sbsec lock.

> > 
> > >  static int superblock_doinit(struct super_block *sb, void *data)
> > >  {
> > >  	struct superblock_security_struct *sbsec = sb->s_security;
> > > -	struct dentry *root = sb->s_root;
> > > -	struct inode *inode = root->d_inode;
> > >  	int rc = 0;
> > >  
> > > -	mutex_lock(&sbsec->lock);
> > >  	if (sbsec->initialized)
> > >  		goto out;
> > 
> > One of the things that needs to be fixed is handling of a context mount
> > option when the superblock has previously been setup.  At present (and
> > still true of your patch), if one mounts /home/eparis with one context
> > mount and then tries to mount /home/sds with a different context mount,
> > the latter proceeds with no indication that the context mount option was
> > ignored (because we immediately return with success if already
> > initialized, and they'll end up with the same superblock if they are
> > from the same filesystem).  So we need to at least check to see if data
> > is set and return an error if sbsec is already initialized so that
> > userspace gets an indication of the fact.
> 
> I pointed out where I think we need to do this, but what kind of error
> do we want to return?  Do we actually want the mount command to fail?

-EINVAL with a printk.  Yes, we want the mount to fail there, because
the user asked for a particular context mount and the kernel can't honor
the request.  The user is always free to then do a regular mount if he
is ok with that.

-- 
Stephen Smalley
National Security Agency


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] 1/2 SELinux: Add get, set, and cloning of superblock security information

[Stephen Smalley]

On Thu, 2007-09-06 at 15:21 -0400, Eric Paris wrote:
> On Thu, 2007-09-06 at 14:33 -0400, Stephen Smalley wrote:
> > On Wed, 2007-09-05 at 18:16 -0400, Eric Paris wrote:
> > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > security_clont_sb_mnt_opts to the LSM and to SELinux.  This is in
> > > preparation for NFS to be able to own its own mount options and remove
> > > the NFS specific code from SELinux.
> > > 
> > > >From the point of view of SELinux NFS does not use text based mount
> > > options so this patch is not something that is going away.  NFS merely
> > > takes the text options from userspace and puts them into the same binary
> > > format as always and passes that binary mount data around.
> > > 
> > > Signed-off-by: Eric Paris <eparis@redhat.com>
> > > 
> > > ---
> > > 
> > >  include/linux/security.h          |   70 +++++
> > >  security/dummy.c                  |   26 ++
> > >  security/selinux/hooks.c          |  618 ++++++++++++++++++++++++-------------
> > >  security/selinux/include/objsec.h |    1 +
> > >  4 files changed, 505 insertions(+), 210 deletions(-)
> > 
> > Not a very nice diff to read.
> 
> I couldn't think of a great way to make it smaller.  I certainly
> understand....
> 
> > 
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> 
> > > +static inline int security_sb_set_mnt_opts (struct super_block *sb,
> > > +					    char **mount_options,
> > > +					    int *flags, int num_opts)
> > > +{
> > > +	return 0;
> > > +}
> > 
> > For consistency, this should likely do the same thing as the dummy
> > function, i.e. return -EOPNOTSUPP if num_opts > 0.  Actually, is this
> > going to get called at all unless num_opts is > 0?
> 
> EOPNOTSUPP seems like the right thing.  selinux=0 and mount nfs with
> context=
> 
> > 
> > > diff --git a/security/dummy.c b/security/dummy.c
> 
> > > +static int dummy_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
> > > +				 int *flags, int num_opts)
> > > +{
> > > +	if (unlikely(num_opts))
> > > +		return -EOPNOTSUPP;
> > > +	return 0;
> > > +}
> > 
> > Possibly just unconditionally return -EOPNOTSUPP, as the hook wouldn't
> > normally get called with no options.
> 
> I'd rather not rely on the FS to have to follow this convention.  Its
> written so that num_opts=0 will work fine.
> > 
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 3694662..5ebe30b 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -362,135 +362,228 @@ static int may_context_mount_inode_relabel(u32 sid,
> > >  	return rc;
> > >  }
> > >  
> > > -static int try_context_mount(struct super_block *sb, void *data)
> > > +static int sb_finish_set_opts(struct super_block *sb)
> > >  {
> > > -	char *context = NULL, *defcontext = NULL;
> > > -	char *fscontext = NULL, *rootcontext = NULL;
> > > -	const char *name;
> > > -	u32 sid;
> > > -	int alloc = 0, rc = 0, seen = 0;
> > > -	struct task_security_struct *tsec = current->security;
> > >  	struct superblock_security_struct *sbsec = sb->s_security;
> > > +	struct dentry *root = sb->s_root;
> > > +	struct inode *root_inode = root->d_inode;
> > > +	int rc = 0;
> > >  
> > > -	if (!data)
> > > -		goto out;
> > > +	BUG_ON(!ss_initialized);
> > 
> > Why BUG_ON vs. return error?  context mount before initial policy load
> > is technically possible, even more so if booting permissive and policy
> > is corrupted.
> 
> How should your example case be handled?  we can't store that context
> information for later.

Error code.

>   The only way we get here is through a direct
> caller to set or clone from the FS since normally we would come through
> superblock_doinit->try_context_mount->set_sb...->here and ss_initialized
> would already have been taken care of.  Should I just dump the
> superblock on the list to deal with later and not worry that I was given
> mount options (doesn't seem like we have a good solution to your problem
> now)?
> 
> I like an error code better, what fits?  It should be dealt with in
> selinux_set_mnt_opts/sb_doinit and not here.  But still the question
> remains.

-EOPNOTSUPP or -EINVAL.  Possibly with a printk.

> > 
> > >  
> > > -	name = sb->s_type->name;
> > > +	if (sbsec->initialized)
> > > +		return 0;
> > 
> > Under what conditions would we reach this point if sbsec was already
> > initialized?  Should it be an error?  A bug?
> 
> We can't.  it is caught in both superblock_doinit and
> selinux_set_sb_mnt_opts.

So BUG_ON?  Or drop it.

> > > +static int try_context_mount(struct super_block *sb, void *data)
> > > +{
> > > +	char *context = NULL, *defcontext = NULL;
> > > +	char *fscontext = NULL, *rootcontext = NULL;
> > > +	int rc = 0;
> > > +	char *p, *options = data;
> > > +	/* selinux only know about 4 mount options */
> > > +	char *mnt_opts[4];
> > > +	int mnt_opts_flags[4], num_mnt_opts = 0;
> > 
> > Can we #define that once and use it?
> 
> the variable declarations?  I guess I could.  They aren't used in many
> other places, but having [4] randomly in the code is probably not the
> best idea.

Yes, I just meant replacing 4 with a #define'd symbol.

> > >  
> > > +/* 
> > > + * no locking done here, but it shouldn't matter, sbsec->proc would just get
> > > + * set the same way in another thread and fs_use, hmmmmmm 
> > > + */
> > 
> > Not sure I follow; you do take the lock still in set_mnts_opts before
> > doing the above AFAICS.  sbsec->list though is another matter.
> 
> comment doesn't belong.  I wasn't taking it and was still setting ->proc
> here but realized it was a stupid idea.  Never removed the comment.

Ok, but sbsec->list might still be an issue technically even if not in
practice.  There is a global spinlock on the whole list, but you could
end up adding the superblock twice if you don't take the sbsec lock.

> > 
> > >  static int superblock_doinit(struct super_block *sb, void *data)
> > >  {
> > >  	struct superblock_security_struct *sbsec = sb->s_security;
> > > -	struct dentry *root = sb->s_root;
> > > -	struct inode *inode = root->d_inode;
> > >  	int rc = 0;
> > >  
> > > -	mutex_lock(&sbsec->lock);
> > >  	if (sbsec->initialized)
> > >  		goto out;
> > 
> > One of the things that needs to be fixed is handling of a context mount
> > option when the superblock has previously been setup.  At present (and
> > still true of your patch), if one mounts /home/eparis with one context
> > mount and then tries to mount /home/sds with a different context mount,
> > the latter proceeds with no indication that the context mount option was
> > ignored (because we immediately return with success if already
> > initialized, and they'll end up with the same superblock if they are
> > from the same filesystem).  So we need to at least check to see if data
> > is set and return an error if sbsec is already initialized so that
> > userspace gets an indication of the fact.
> 
> I pointed out where I think we need to do this, but what kind of error
> do we want to return?  Do we actually want the mount command to fail?

-EINVAL with a printk.  Yes, we want the mount to fail there, because
the user asked for a particular context mount and the kernel can't honor
the request.  The user is always free to then do a regular mount if he
is ok with that.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.