[PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

[BUG]
Originally when mount btrfs with "-o subvol=" mount option, btrfs will
lose all security lable.
And if the btrfs fs is mounted somewhere else, due to the lost of
security lable, SELinux will refuse to mount since the same super block
is being mounted using different security lable.

[REPRODUCER]
With SELinux enabled:
 #mkfs -t btrfs /dev/sda5
 #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
 #btrfs subvolume create /mnt/btrfs/subvol
 #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
  /mnt/test

kernel message:
SELinux: mount invalid.  Same superblock, different security settings
for (dev sda5, type btrfs)

[REASON]
This happens because btrfs will call vfs_kern_mount() and then
mount_subtree() to handle subvolume name lookup.
First mount will cut off all the security lables and when it comes to
the second vfs_kern_mount(), it has no security label now.

[FIX]
This patch will makes btrfs behavior much more like nfs,
which has the type flag FS_BINARY_MOUNTDATA,
making btrfs handles the security label internally.
So security label will be set in the real mount time and won't lose
label when use with "subvol=" mount option.

Reported-by: Eryu Guan <guaneryu@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/ctree.h |  5 +++
 fs/btrfs/super.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8e29b61..c82dd6d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -34,6 +34,7 @@
 #include <linux/pagemap.h>
 #include <linux/btrfs.h>
 #include <linux/workqueue.h>
+#include <linux/security.h>
 #include "extent_io.h"
 #include "extent_map.h"
 #include "async-thread.h"
@@ -1723,6 +1724,9 @@ struct btrfs_fs_info {
 
 	/* Used to reclaim the metadata space in the background. */
 	struct work_struct async_reclaim_work;
+
+	/* For btrfs to record security options */
+	struct security_mnt_opts security_opts;
 };
 
 struct btrfs_subvolume_writers {
@@ -3604,6 +3608,7 @@ static inline void free_fs_info(struct btrfs_fs_info *fs_info)
 	kfree(fs_info->uuid_root);
 	kfree(fs_info->super_copy);
 	kfree(fs_info->super_for_commit);
+	security_free_mnt_opts(&fs_info->security_opts);
 	kfree(fs_info);
 }
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index c4124de..1eb7858 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1215,6 +1215,54 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags,
 	return root;
 }
 
+static int parse_security_options(char *orig_opts,
+				  struct security_mnt_opts *sec_opts)
+{
+	char *secdata = NULL;
+	int ret = 0;
+
+	secdata = alloc_secdata();
+	if (!secdata)
+		return -ENOMEM;
+	ret = security_sb_copy_data(orig_opts, secdata);
+	if (ret) {
+		free_secdata(secdata);
+		return ret;
+	}
+	ret = security_sb_parse_opts_str(secdata, sec_opts);
+	free_secdata(secdata);
+	return ret;
+}
+
+static int setup_security_options(struct btrfs_fs_info *fs_info,
+				  struct super_block *sb,
+				  struct security_mnt_opts *sec_opts)
+{
+	int ret = 0;
+
+	/*
+	 * Call security_sb_set_mnt_opts() to check whether new sec_opts
+	 * is valid.
+	 */
+	ret = security_sb_set_mnt_opts(sb, sec_opts, 0, NULL);
+	if (ret)
+		return ret;
+
+	if (!fs_info->security_opts.num_mnt_opts) {
+		/* first time security setup, copy sec_opts to fs_info */
+		memcpy(&fs_info->security_opts, sec_opts, sizeof(*sec_opts));
+	} else {
+		/*
+		 * Since SELinux(the only one supports security_mnt_opts) does
+		 * NOT support changing context during remount/mount same sb,
+		 * This must be the same or part of the same security options,
+		 * just free it.
+		 */
+		security_free_mnt_opts(sec_opts);
+	}
+	return ret;
+}
+
 /*
  * Find a superblock for the given device / mount point.
  *
@@ -1229,6 +1277,7 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
 	struct dentry *root;
 	struct btrfs_fs_devices *fs_devices = NULL;
 	struct btrfs_fs_info *fs_info = NULL;
+	struct security_mnt_opts new_sec_opts;
 	fmode_t mode = FMODE_READ;
 	char *subvol_name = NULL;
 	u64 subvol_objectid = 0;
@@ -1251,9 +1300,16 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
 		return root;
 	}
 
+	security_init_mnt_opts(&new_sec_opts);
+	if (data) {
+		error = parse_security_options(data, &new_sec_opts);
+		if (error)
+			return ERR_PTR(error);
+	}
+
 	error = btrfs_scan_one_device(device_name, mode, fs_type, &fs_devices);
 	if (error)
-		return ERR_PTR(error);
+		goto error_sec_opts;
 
 	/*
 	 * Setup a dummy root and fs_info for test/set super.  This is because
@@ -1262,13 +1318,16 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
 	 * then open_ctree will properly initialize everything later.
 	 */
 	fs_info = kzalloc(sizeof(struct btrfs_fs_info), GFP_NOFS);
-	if (!fs_info)
-		return ERR_PTR(-ENOMEM);
+	if (!fs_info) {
+		error = -ENOMEM;
+		goto error_sec_opts;
+	}
 
 	fs_info->fs_devices = fs_devices;
 
 	fs_info->super_copy = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_NOFS);
 	fs_info->super_for_commit = kzalloc(BTRFS_SUPER_INFO_SIZE, GFP_NOFS);
+	security_init_mnt_opts(&fs_info->security_opts);
 	if (!fs_info->super_copy || !fs_info->super_for_commit) {
 		error = -ENOMEM;
 		goto error_fs_info;
@@ -1306,8 +1365,19 @@ static struct dentry *btrfs_mount(struct file_system_type *fs_type, int flags,
 	}
 
 	root = !error ? get_default_root(s, subvol_objectid) : ERR_PTR(error);
-	if (IS_ERR(root))
+	if (IS_ERR(root)) {
+		deactivate_locked_super(s);
+		error = PTR_ERR(root);
+		goto error_sec_opts;
+	}
+
+	fs_info = btrfs_sb(s);
+	error = setup_security_options(fs_info, s, &new_sec_opts);
+	if (error) {
+		dput(root);
 		deactivate_locked_super(s);
+		goto error_sec_opts;
+	}
 
 	return root;
 
@@ -1315,6 +1385,8 @@ error_close_devices:
 	btrfs_close_devices(fs_devices);
 error_fs_info:
 	free_fs_info(fs_info);
+error_sec_opts:
+	security_free_mnt_opts(&new_sec_opts);
 	return ERR_PTR(error);
 }
 
@@ -1396,6 +1468,21 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 	sync_filesystem(sb);
 	btrfs_remount_prepare(fs_info);
 
+	if (data) {
+		struct security_mnt_opts new_sec_opts;
+
+		security_init_mnt_opts(&new_sec_opts);
+		ret = parse_security_options(data, &new_sec_opts);
+		if (ret)
+			goto restore;
+		ret = setup_security_options(fs_info, sb,
+					     &new_sec_opts);
+		if (ret) {
+			security_free_mnt_opts(&new_sec_opts);
+			goto restore;
+		}
+	}
+
 	ret = btrfs_parse_options(root, data);
 	if (ret) {
 		ret = -EINVAL;
@@ -1769,7 +1856,7 @@ static struct file_system_type btrfs_fs_type = {
 	.name		= "btrfs",
 	.mount		= btrfs_mount,
 	.kill_sb	= btrfs_kill_super,
-	.fs_flags	= FS_REQUIRES_DEV,
+	.fs_flags	= FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
 };
 MODULE_ALIAS_FS("btrfs");
 
-- 
2.1.0

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Chris Mason]

On 09/23/2014 01:40 AM, Qu Wenruo wrote:
> [BUG]
> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
> lose all security lable.
> And if the btrfs fs is mounted somewhere else, due to the lost of
> security lable, SELinux will refuse to mount since the same super block
> is being mounted using different security lable.
> 
> [REPRODUCER]
> With SELinux enabled:
>  #mkfs -t btrfs /dev/sda5
>  #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>  #btrfs subvolume create /mnt/btrfs/subvol
>  #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>   /mnt/test
> 
> kernel message:
> SELinux: mount invalid.  Same superblock, different security settings
> for (dev sda5, type btrfs)
> 
> [REASON]
> This happens because btrfs will call vfs_kern_mount() and then
> mount_subtree() to handle subvolume name lookup.
> First mount will cut off all the security lables and when it comes to
> the second vfs_kern_mount(), it has no security label now.
> 
> [FIX]
> This patch will makes btrfs behavior much more like nfs,
> which has the type flag FS_BINARY_MOUNTDATA,
> making btrfs handles the security label internally.
> So security label will be set in the real mount time and won't lose
> label when use with "subvol=" mount option.

Thanks for working on this.  Eric Sandeen (cc'd) was trying out
something similar recently, so I want to make sure this doesn't conflict
with his ideas.

-chris

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Eric Sandeen]

On 9/23/14 7:49 AM, Chris Mason wrote:
> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>> [BUG]
>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>> lose all security lable.
>> And if the btrfs fs is mounted somewhere else, due to the lost of
>> security lable, SELinux will refuse to mount since the same super block
>> is being mounted using different security lable.
>>
>> [REPRODUCER]
>> With SELinux enabled:
>>  #mkfs -t btrfs /dev/sda5
>>  #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>  #btrfs subvolume create /mnt/btrfs/subvol
>>  #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>   /mnt/test
>>
>> kernel message:
>> SELinux: mount invalid.  Same superblock, different security settings
>> for (dev sda5, type btrfs)
>>
>> [REASON]
>> This happens because btrfs will call vfs_kern_mount() and then
>> mount_subtree() to handle subvolume name lookup.
>> First mount will cut off all the security lables and when it comes to
>> the second vfs_kern_mount(), it has no security label now.
>>
>> [FIX]
>> This patch will makes btrfs behavior much more like nfs,
>> which has the type flag FS_BINARY_MOUNTDATA,
>> making btrfs handles the security label internally.
>> So security label will be set in the real mount time and won't lose
>> label when use with "subvol=" mount option.
> 
> Thanks for working on this.  Eric Sandeen (cc'd) was trying out
> something similar recently, so I want to make sure this doesn't conflict
> with his ideas.

My ideas didn't get very far.  ;)

What I was after was a way for multiple subvolumes to have unique contexts.
It looks like this might do the trick, as long as they are mounted on a unique
mount point.

Would this allow "subvolume create" to take a context, so that everything
under /mnt/btrfs/subvol/ has a unique subvol-wide context?

thanks,
-Eric

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options 
internally to avoid losing security label.
From: Eric Sandeen <sandeen@redhat.com>
To: Chris Mason <clm@fb.com>, Qu Wenruo <quwenruo@cn.fujitsu.com>, 
<linux-btrfs@vger.kernel.org>
Date: 2014年09月24日 02:51
> On 9/23/14 7:49 AM, Chris Mason wrote:
>> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>>> [BUG]
>>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>>> lose all security lable.
>>> And if the btrfs fs is mounted somewhere else, due to the lost of
>>> security lable, SELinux will refuse to mount since the same super block
>>> is being mounted using different security lable.
>>>
>>> [REPRODUCER]
>>> With SELinux enabled:
>>>   #mkfs -t btrfs /dev/sda5
>>>   #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>>   #btrfs subvolume create /mnt/btrfs/subvol
>>>   #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>>    /mnt/test
>>>
>>> kernel message:
>>> SELinux: mount invalid.  Same superblock, different security settings
>>> for (dev sda5, type btrfs)
>>>
>>> [REASON]
>>> This happens because btrfs will call vfs_kern_mount() and then
>>> mount_subtree() to handle subvolume name lookup.
>>> First mount will cut off all the security lables and when it comes to
>>> the second vfs_kern_mount(), it has no security label now.
>>>
>>> [FIX]
>>> This patch will makes btrfs behavior much more like nfs,
>>> which has the type flag FS_BINARY_MOUNTDATA,
>>> making btrfs handles the security label internally.
>>> So security label will be set in the real mount time and won't lose
>>> label when use with "subvol=" mount option.
>> Thanks for working on this.  Eric Sandeen (cc'd) was trying out
>> something similar recently, so I want to make sure this doesn't conflict
>> with his ideas.
> My ideas didn't get very far.  ;)
>
> What I was after was a way for multiple subvolumes to have unique contexts.
> It looks like this might do the trick, as long as they are mounted on a unique
> mount point.
>
> Would this allow "subvolume create" to take a context, so that everything
> under /mnt/btrfs/subvol/ has a unique subvol-wide context?
>
> thanks,
> -Eric
Did you mean the following situation?
/dev/sdb default subvol(FS_TREE) mounted on /mnt/default with context A
/dev/sdb subvol=subvol mounted on /mnt/subvol with context B

If that's your goal, I am afraid that my patch can't achieve it and even 
worse, will even forbid it. :(

SELinux doesn't allow same superblock mounted with different context, 
and the patch follows it.
If SELinux is modified to allow same superblock different context, then 
my patch also needs to be modified.

Thanks,
Qu

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Eric Sandeen]

On 9/23/14 7:31 PM, Qu Wenruo wrote:
> 
> -------- Original Message --------
> Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.
> From: Eric Sandeen <sandeen@redhat.com>
> To: Chris Mason <clm@fb.com>, Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org>
> Date: 2014年09月24日 02:51
>> On 9/23/14 7:49 AM, Chris Mason wrote:
>>> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>>>> [BUG]
>>>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>>>> lose all security lable.
>>>> And if the btrfs fs is mounted somewhere else, due to the lost of
>>>> security lable, SELinux will refuse to mount since the same super block
>>>> is being mounted using different security lable.
>>>>
>>>> [REPRODUCER]
>>>> With SELinux enabled:
>>>>   #mkfs -t btrfs /dev/sda5
>>>>   #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>>>   #btrfs subvolume create /mnt/btrfs/subvol
>>>>   #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>>>    /mnt/test
>>>>
>>>> kernel message:
>>>> SELinux: mount invalid.  Same superblock, different security settings
>>>> for (dev sda5, type btrfs)
>>>>
>>>> [REASON]
>>>> This happens because btrfs will call vfs_kern_mount() and then
>>>> mount_subtree() to handle subvolume name lookup.
>>>> First mount will cut off all the security lables and when it comes to
>>>> the second vfs_kern_mount(), it has no security label now.
>>>>
>>>> [FIX]
>>>> This patch will makes btrfs behavior much more like nfs,
>>>> which has the type flag FS_BINARY_MOUNTDATA,
>>>> making btrfs handles the security label internally.
>>>> So security label will be set in the real mount time and won't lose
>>>> label when use with "subvol=" mount option.
>>> Thanks for working on this.  Eric Sandeen (cc'd) was trying out
>>> something similar recently, so I want to make sure this doesn't conflict
>>> with his ideas.
>> My ideas didn't get very far.  ;)
>>
>> What I was after was a way for multiple subvolumes to have unique contexts.
>> It looks like this might do the trick, as long as they are mounted on a unique
>> mount point.
>>
>> Would this allow "subvolume create" to take a context, so that everything
>> under /mnt/btrfs/subvol/ has a unique subvol-wide context?
>>
>> thanks,
>> -Eric
> Did you mean the following situation?
> /dev/sdb default subvol(FS_TREE) mounted on /mnt/default with context A
> /dev/sdb subvol=subvol mounted on /mnt/subvol with context B
> 
> If that's your goal, I am afraid that my patch can't achieve it and even worse, will even forbid it. :(
> 
> SELinux doesn't allow same superblock mounted with different context, and the patch follows it.
> If SELinux is modified to allow same superblock different context, then my patch also needs to be modified.

oh, ok, I see.

I don't think that my "wish" should disallow your patch.

For the problem I was looking at, I think the only way forward would require
some significant selinux modification, and treating a subvol root essentially
like a superblock.

So ... don't let me slow you down, at least for now.  ;)

-Eric

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options 
internally to avoid losing security label.
From: Eric Sandeen <sandeen@redhat.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, Chris Mason <clm@fb.com>, 
<linux-btrfs@vger.kernel.org>
Date: 2014年09月24日 11:33
> On 9/23/14 7:31 PM, Qu Wenruo wrote:
>> -------- Original Message --------
>> Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.
>> From: Eric Sandeen <sandeen@redhat.com>
>> To: Chris Mason <clm@fb.com>, Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org>
>> Date: 2014年09月24日 02:51
>>> On 9/23/14 7:49 AM, Chris Mason wrote:
>>>> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>>>>> [BUG]
>>>>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>>>>> lose all security lable.
>>>>> And if the btrfs fs is mounted somewhere else, due to the lost of
>>>>> security lable, SELinux will refuse to mount since the same super block
>>>>> is being mounted using different security lable.
>>>>>
>>>>> [REPRODUCER]
>>>>> With SELinux enabled:
>>>>>    #mkfs -t btrfs /dev/sda5
>>>>>    #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>>>>    #btrfs subvolume create /mnt/btrfs/subvol
>>>>>    #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>>>>     /mnt/test
>>>>>
>>>>> kernel message:
>>>>> SELinux: mount invalid.  Same superblock, different security settings
>>>>> for (dev sda5, type btrfs)
>>>>>
>>>>> [REASON]
>>>>> This happens because btrfs will call vfs_kern_mount() and then
>>>>> mount_subtree() to handle subvolume name lookup.
>>>>> First mount will cut off all the security lables and when it comes to
>>>>> the second vfs_kern_mount(), it has no security label now.
>>>>>
>>>>> [FIX]
>>>>> This patch will makes btrfs behavior much more like nfs,
>>>>> which has the type flag FS_BINARY_MOUNTDATA,
>>>>> making btrfs handles the security label internally.
>>>>> So security label will be set in the real mount time and won't lose
>>>>> label when use with "subvol=" mount option.
>>>> Thanks for working on this.  Eric Sandeen (cc'd) was trying out
>>>> something similar recently, so I want to make sure this doesn't conflict
>>>> with his ideas.
>>> My ideas didn't get very far.  ;)
>>>
>>> What I was after was a way for multiple subvolumes to have unique contexts.
>>> It looks like this might do the trick, as long as they are mounted on a unique
>>> mount point.
>>>
>>> Would this allow "subvolume create" to take a context, so that everything
>>> under /mnt/btrfs/subvol/ has a unique subvol-wide context?
>>>
>>> thanks,
>>> -Eric
>> Did you mean the following situation?
>> /dev/sdb default subvol(FS_TREE) mounted on /mnt/default with context A
>> /dev/sdb subvol=subvol mounted on /mnt/subvol with context B
>>
>> If that's your goal, I am afraid that my patch can't achieve it and even worse, will even forbid it. :(
>>
>> SELinux doesn't allow same superblock mounted with different context, and the patch follows it.
>> If SELinux is modified to allow same superblock different context, then my patch also needs to be modified.
> oh, ok, I see.
>
> I don't think that my "wish" should disallow your patch.
>
> For the problem I was looking at, I think the only way forward would require
> some significant selinux modification, and treating a subvol root essentially
> like a superblock.
>
> So ... don't let me slow you down, at least for now.  ;)
>
> -Eric
>
BTW, since with the patch btrfs can in fact don't call 
security_sb_set_mnt_opts() if btrfs wants,
what about set context to the dentry or something like that, but not set 
to superblock?
I may be wrong, since I am still not famaliar with security parts...

Thanks,
Qu

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options 
internally to avoid losing security label.
From: Eric Sandeen <sandeen@redhat.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, Chris Mason <clm@fb.com>, 
<linux-btrfs@vger.kernel.org>
Date: 2014年09月24日 11:33
> On 9/23/14 7:31 PM, Qu Wenruo wrote:
>> -------- Original Message --------
>> Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.
>> From: Eric Sandeen <sandeen@redhat.com>
>> To: Chris Mason <clm@fb.com>, Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org>
>> Date: 2014年09月24日 02:51
>>> On 9/23/14 7:49 AM, Chris Mason wrote:
>>>> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>>>>> [BUG]
>>>>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>>>>> lose all security lable.
>>>>> And if the btrfs fs is mounted somewhere else, due to the lost of
>>>>> security lable, SELinux will refuse to mount since the same super block
>>>>> is being mounted using different security lable.
>>>>>
>>>>> [REPRODUCER]
>>>>> With SELinux enabled:
>>>>>    #mkfs -t btrfs /dev/sda5
>>>>>    #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>>>>    #btrfs subvolume create /mnt/btrfs/subvol
>>>>>    #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>>>>     /mnt/test
>>>>>
>>>>> kernel message:
>>>>> SELinux: mount invalid.  Same superblock, different security settings
>>>>> for (dev sda5, type btrfs)
>>>>>
>>>>> [REASON]
>>>>> This happens because btrfs will call vfs_kern_mount() and then
>>>>> mount_subtree() to handle subvolume name lookup.
>>>>> First mount will cut off all the security lables and when it comes to
>>>>> the second vfs_kern_mount(), it has no security label now.
>>>>>
>>>>> [FIX]
>>>>> This patch will makes btrfs behavior much more like nfs,
>>>>> which has the type flag FS_BINARY_MOUNTDATA,
>>>>> making btrfs handles the security label internally.
>>>>> So security label will be set in the real mount time and won't lose
>>>>> label when use with "subvol=" mount option.
>>>> Thanks for working on this.  Eric Sandeen (cc'd) was trying out
>>>> something similar recently, so I want to make sure this doesn't conflict
>>>> with his ideas.
>>> My ideas didn't get very far.  ;)
>>>
>>> What I was after was a way for multiple subvolumes to have unique contexts.
>>> It looks like this might do the trick, as long as they are mounted on a unique
>>> mount point.
>>>
>>> Would this allow "subvolume create" to take a context, so that everything
>>> under /mnt/btrfs/subvol/ has a unique subvol-wide context?
>>>
>>> thanks,
>>> -Eric
>> Did you mean the following situation?
>> /dev/sdb default subvol(FS_TREE) mounted on /mnt/default with context A
>> /dev/sdb subvol=subvol mounted on /mnt/subvol with context B
>>
>> If that's your goal, I am afraid that my patch can't achieve it and even worse, will even forbid it. :(
>>
>> SELinux doesn't allow same superblock mounted with different context, and the patch follows it.
>> If SELinux is modified to allow same superblock different context, then my patch also needs to be modified.
> oh, ok, I see.
>
> I don't think that my "wish" should disallow your patch.
>
> For the problem I was looking at, I think the only way forward would require
> some significant selinux modification, and treating a subvol root essentially
> like a superblock.
>
> So ... don't let me slow you down, at least for now.  ;)
>
> -Eric
>
To Chris, any other comment?

Thanks,
Qu

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Chris Mason]

On Sun, Oct 5, 2014 at 11:02 PM, Qu Wenruo <quwenruo@cn.fujitsu.com> 
wrote:
> 
>> 
>> I don't think that my "wish" should disallow your patch.
>> 
>> For the problem I was looking at, I think the only way forward would 
>> require
>> some significant selinux modification, and treating a subvol root 
>> essentially
>> like a superblock.
>> 
>> So ... don't let me slow you down, at least for now.  ;)
>> 
>> -Eric
>> 
> To Chris, any other comment?

It looks good to me, and I have it queued up here in testing.

-chris

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options 
internally to avoid losing security label.
From: Chris Mason <clm@fb.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年10月06日 21:26
> On Sun, Oct 5, 2014 at 11:02 PM, Qu Wenruo <quwenruo@cn.fujitsu.com> 
> wrote:
>>
>>>
>>> I don't think that my "wish" should disallow your patch.
>>>
>>> For the problem I was looking at, I think the only way forward would 
>>> require
>>> some significant selinux modification, and treating a subvol root 
>>> essentially
>>> like a superblock.
>>>
>>> So ... don't let me slow you down, at least for now.  ;)
>>>
>>> -Eric
>>>
>> To Chris, any other comment?
>
> It looks good to me, and I have it queued up here in testing.
>
> -chris
>
>
>
Thanks a lot.

Qu

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Josef Bacik]

On 09/23/2014 01:40 AM, Qu Wenruo wrote:
> [BUG]
> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
> lose all security lable.
> And if the btrfs fs is mounted somewhere else, due to the lost of
> security lable, SELinux will refuse to mount since the same super block
> is being mounted using different security lable.
>
> [REPRODUCER]
> With SELinux enabled:
>   #mkfs -t btrfs /dev/sda5
>   #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>   #btrfs subvolume create /mnt/btrfs/subvol
>   #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>    /mnt/test
>
> kernel message:
> SELinux: mount invalid.  Same superblock, different security settings
> for (dev sda5, type btrfs)
>
> [REASON]
> This happens because btrfs will call vfs_kern_mount() and then
> mount_subtree() to handle subvolume name lookup.
> First mount will cut off all the security lables and when it comes to
> the second vfs_kern_mount(), it has no security label now.
>
> [FIX]
> This patch will makes btrfs behavior much more like nfs,
> which has the type flag FS_BINARY_MOUNTDATA,
> making btrfs handles the security label internally.
> So security label will be set in the real mount time and won't lose
> label when use with "subvol=" mount option.
>

Please make this an xfstest, I'm going to change how subvols are mounted 
in a bit and I'd like to make sure I don't break anything.  Thanks,

Josef

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Eryu Guan]

On Mon, Oct 06, 2014 at 09:29:25AM -0400, Josef Bacik wrote:
> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
> >[BUG]
> >Originally when mount btrfs with "-o subvol=" mount option, btrfs will
> >lose all security lable.
> >And if the btrfs fs is mounted somewhere else, due to the lost of
> >security lable, SELinux will refuse to mount since the same super block
> >is being mounted using different security lable.
> >
> >[REPRODUCER]
> >With SELinux enabled:
> >  #mkfs -t btrfs /dev/sda5
> >  #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
> >  #btrfs subvolume create /mnt/btrfs/subvol
> >  #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
> >   /mnt/test
> >
> >kernel message:
> >SELinux: mount invalid.  Same superblock, different security settings
> >for (dev sda5, type btrfs)
> >
> >[REASON]
> >This happens because btrfs will call vfs_kern_mount() and then
> >mount_subtree() to handle subvolume name lookup.
> >First mount will cut off all the security lables and when it comes to
> >the second vfs_kern_mount(), it has no security label now.
> >
> >[FIX]
> >This patch will makes btrfs behavior much more like nfs,
> >which has the type flag FS_BINARY_MOUNTDATA,
> >making btrfs handles the security label internally.
> >So security label will be set in the real mount time and won't lose
> >label when use with "subvol=" mount option.
> >
> 
> Please make this an xfstest, I'm going to change how subvols are mounted in
> a bit and I'd like to make sure I don't break anything.  Thanks,

Hi Qu, I'll submit one xfstest, just want to make sure you don't do
duplicated work here.

Thanks,
Eryu

Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options 
internally to avoid losing security label.
From: Eryu Guan <guaneryu@gmail.com>
To: <quwenruo@cn.fujitsu.com>
Date: 2014年10月06日 21:38
> On Mon, Oct 06, 2014 at 09:29:25AM -0400, Josef Bacik wrote:
>> On 09/23/2014 01:40 AM, Qu Wenruo wrote:
>>> [BUG]
>>> Originally when mount btrfs with "-o subvol=" mount option, btrfs will
>>> lose all security lable.
>>> And if the btrfs fs is mounted somewhere else, due to the lost of
>>> security lable, SELinux will refuse to mount since the same super block
>>> is being mounted using different security lable.
>>>
>>> [REPRODUCER]
>>> With SELinux enabled:
>>>   #mkfs -t btrfs /dev/sda5
>>>   #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs
>>>   #btrfs subvolume create /mnt/btrfs/subvol
>>>   #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5
>>>    /mnt/test
>>>
>>> kernel message:
>>> SELinux: mount invalid.  Same superblock, different security settings
>>> for (dev sda5, type btrfs)
>>>
>>> [REASON]
>>> This happens because btrfs will call vfs_kern_mount() and then
>>> mount_subtree() to handle subvolume name lookup.
>>> First mount will cut off all the security lables and when it comes to
>>> the second vfs_kern_mount(), it has no security label now.
>>>
>>> [FIX]
>>> This patch will makes btrfs behavior much more like nfs,
>>> which has the type flag FS_BINARY_MOUNTDATA,
>>> making btrfs handles the security label internally.
>>> So security label will be set in the real mount time and won't lose
>>> label when use with "subvol=" mount option.
>>>
>> Please make this an xfstest, I'm going to change how subvols are mounted in
>> a bit and I'd like to make sure I don't break anything.  Thanks,
> Hi Qu, I'll submit one xfstest, just want to make sure you don't do
> duplicated work here.
>
> Thanks,
> Eryu
Thanks a lot.

I remember you have already submitted an xfstest testcase for this.

Thanks,
Qu