diff mbox series

[RFC,11/10] selinux: fix support multiple selinuxfs instances

Message ID 20191016182815.3188-1-sds@tycho.nsa.gov (mailing list archive)
State Accepted
Headers show
Series None | expand

Commit Message

Stephen Smalley Oct. 16, 2019, 6:28 p.m. UTC
When I rebased the "selinux: support multiple selinuxfs instances"
patch, I misunderstood the vfs get_tree_keyed() interface and thought
it eliminated the need for our own custom get_tree() implementation.
However, it turns out that we cannot use get_tree_keyed() because we
are already using sb->s_fs_info to store per-instance data and
get_tree_keyed() uses it for the key.  The resulting behavior was
that a unique superblock was being created for every selinuxfs mount,
even within the same selinux namespace, which is not desirable.
Restore the old logic adapted for the relevant vfs changes.

This should likely be squashed into the original commit before final
upstreaming.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 security/selinux/selinuxfs.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Stephen Smalley Oct. 17, 2019, 12:04 p.m. UTC | #1
On 10/16/19 2:28 PM, Stephen Smalley wrote:
> When I rebased the "selinux: support multiple selinuxfs instances"
> patch, I misunderstood the vfs get_tree_keyed() interface and thought
> it eliminated the need for our own custom get_tree() implementation.
> However, it turns out that we cannot use get_tree_keyed() because we
> are already using sb->s_fs_info to store per-instance data and
> get_tree_keyed() uses it for the key.  The resulting behavior was
> that a unique superblock was being created for every selinuxfs mount,
> even within the same selinux namespace, which is not desirable.
> Restore the old logic adapted for the relevant vfs changes.
> 
> This should likely be squashed into the original commit before final
> upstreaming.

FYI, I squashed this into the original commit introducing multiple 
selinuxfs instances and pushed it to the branch.

> 
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>   security/selinux/selinuxfs.c | 26 +++++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 1ba4d874fc86..6bb0c1611e6f 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -2102,9 +2102,33 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
>   	return ret;
>   }
>   
> +static int selinuxfs_compare(struct super_block *sb, struct fs_context *fc)
> +{
> +	struct selinux_fs_info *fsi = sb->s_fs_info;
> +
> +	return (current_selinux_ns == fsi->ns);
> +}
> +
>   static int sel_get_tree(struct fs_context *fc)
>   {
> -	return get_tree_keyed(fc, sel_fill_super, current_selinux_ns);
> +	struct super_block *sb;
> +	int err;
> +
> +	sb = sget_fc(fc, selinuxfs_compare, set_anon_super_fc);
> +	if (IS_ERR(sb))
> +		return PTR_ERR(sb);
> +
> +	if (!sb->s_root) {
> +		err = sel_fill_super(sb, fc);
> +		if (err) {
> +			deactivate_locked_super(sb);
> +			return err;
> +		}
> +		sb->s_flags |= SB_ACTIVE;
> +	}
> +
> +	fc->root = dget(sb->s_root);
> +	return 0;
>   }
>   
>   static const struct fs_context_operations sel_context_ops = {
>
diff mbox series

Patch

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 1ba4d874fc86..6bb0c1611e6f 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -2102,9 +2102,33 @@  static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
 	return ret;
 }
 
+static int selinuxfs_compare(struct super_block *sb, struct fs_context *fc)
+{
+	struct selinux_fs_info *fsi = sb->s_fs_info;
+
+	return (current_selinux_ns == fsi->ns);
+}
+
 static int sel_get_tree(struct fs_context *fc)
 {
-	return get_tree_keyed(fc, sel_fill_super, current_selinux_ns);
+	struct super_block *sb;
+	int err;
+
+	sb = sget_fc(fc, selinuxfs_compare, set_anon_super_fc);
+	if (IS_ERR(sb))
+		return PTR_ERR(sb);
+
+	if (!sb->s_root) {
+		err = sel_fill_super(sb, fc);
+		if (err) {
+			deactivate_locked_super(sb);
+			return err;
+		}
+		sb->s_flags |= SB_ACTIVE;
+	}
+
+	fc->root = dget(sb->s_root);
+	return 0;
 }
 
 static const struct fs_context_operations sel_context_ops = {