[RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
This commit has the following problem:
1) Break the ro mount rule.
When users mount the whole btrfs ro, it is still possible to mount
subvol rw and change the contents. Which make the whole fs ro mount
non-sense.

2) Cause whole btrfs ro/rw mount change fails.
When mount a subvol ro first, when you can't mount the whole fs mounted
rw. This is due to the check in btrfs_mount() which returns -EBUSY,
which is OK for single fs to prevent mount fs ro in one mount point and
mount the same fs rw in other mount point.
Step to reproduce:
mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail

3) Kernel warn in vfs.
When mount the whole fs ro, and mount a subvol ro, kernel warning will
show in fs/sync.c complaining s_umount rwsem is not locked.
Since this remount is not called by VFS, so s_mounts rwsem is not
correctly locked.

4) Lacks ro/rw conflicint check.
The commit uses a trick to 'cheat' vfs about ro/rw flags to allow
different ro/rw mount options, however this breaks the original ro/rw
conflicting rule: one fs(even subvolume) can't be mounted rw in one place
and mounted ro in someplace else.
This can be triggered quite easily:
mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
mount -o subvol=subv,rw /dev/sda6 /mnt/other

Also the check logical about checking the ro/rw needed to be investigated
further. e.g. When the subvolume is mounted ro, the whole btrfs fs is mounted
rw, so one can change the ro mounted subvolume from the mounted whole btrfs.

Due to the above reasons, the per-subvolume ro/rw mount options should be
investigated further to ensure the correctness and better reverting it
before correct implement
(It may takes a lone time to find the right logic about ro/rw option in
subvolume)

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Conflicts:
	fs/btrfs/super.c
---
 fs/btrfs/super.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4662d92..4a088f8 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -66,8 +66,6 @@
 static const struct super_operations btrfs_super_ops;
 static struct file_system_type btrfs_fs_type;
 
-static int btrfs_remount(struct super_block *sb, int *flags, char *data);
-
 static const char *btrfs_decode_error(int errno)
 {
 	char *errstr = "unknown";
@@ -1179,31 +1177,7 @@ static struct dentry *mount_subvol(const char *subvol_name, int flags,
 		return ERR_PTR(-ENOMEM);
 	mnt = vfs_kern_mount(&btrfs_fs_type, flags, device_name,
 			     newargs);
-
-	if (PTR_RET(mnt) == -EBUSY) {
-		if (flags & MS_RDONLY) {
-			mnt = vfs_kern_mount(&btrfs_fs_type, flags & ~MS_RDONLY, device_name,
-					     newargs);
-		} else {
-			int r;
-			mnt = vfs_kern_mount(&btrfs_fs_type, flags | MS_RDONLY, device_name,
-					     newargs);
-			if (IS_ERR(mnt)) {
-				kfree(newargs);
-				return ERR_CAST(mnt);
-			}
-
-			r = btrfs_remount(mnt->mnt_sb, &flags, NULL);
-			if (r < 0) {
-				/* FIXME: release vfsmount mnt ??*/
-				kfree(newargs);
-				return ERR_PTR(r);
-			}
-		}
-	}
-
 	kfree(newargs);
-
 	if (IS_ERR(mnt))
 		return ERR_CAST(mnt);
 
-- 
2.0.1

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[David Sterba]

(adding Harald to CC)

On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
> This commit has the following problem:
> 1) Break the ro mount rule.
> When users mount the whole btrfs ro, it is still possible to mount
> subvol rw and change the contents. Which make the whole fs ro mount
> non-sense.

The proposed usecase was to allow mounting subvolumes with different
ro/rw flags, and this makes sense to me (provided that the whole
filesystem is mounted rw).

Anything else seems to lead to all the internal problems you point
below. I'm not even sure if mounting the first subvolume 'ro' should
imply that the whole filesystem is ro or not.

> 2) Cause whole btrfs ro/rw mount change fails.
> When mount a subvol ro first, when you can't mount the whole fs mounted
> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
> which is OK for single fs to prevent mount fs ro in one mount point and
> mount the same fs rw in other mount point.
> Step to reproduce:
> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail

Yeah, so first ro means whole filesystem is ro.

> 3) Kernel warn in vfs.
> When mount the whole fs ro, and mount a subvol ro, kernel warning will
> show in fs/sync.c complaining s_umount rwsem is not locked.
> Since this remount is not called by VFS, so s_mounts rwsem is not
> correctly locked.

That's serious.

> 4) Lacks ro/rw conflicint check.
> The commit uses a trick to 'cheat' vfs about ro/rw flags to allow
> different ro/rw mount options, however this breaks the original ro/rw
> conflicting rule: one fs(even subvolume) can't be mounted rw in one place
> and mounted ro in someplace else.
> This can be triggered quite easily:
> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
> mount -o subvol=subv,rw /dev/sda6 /mnt/other

Hm, and that's actually what the bind mount is for, and this was
mentioned in the patch changelog as a workaround.

> Also the check logical about checking the ro/rw needed to be investigated
> further. e.g. When the subvolume is mounted ro, the whole btrfs fs is mounted
> rw, so one can change the ro mounted subvolume from the mounted whole btrfs.

I agree, the semantics is not clear and the code does not cover all
cases.

> Due to the above reasons, the per-subvolume ro/rw mount options should be
> investigated further to ensure the correctness and better reverting it
> before correct implement
> (It may takes a lone time to find the right logic about ro/rw option in
> subvolume)

The different ro/rw feature is convenient, but unless the implementation
covers all the points above, I'm afraid that the revert is the best
option right now.

Ack for the revert. (Yeah I was pushing the patch upstream but ...)

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Chris Mason]

On 07/01/2014 11:32 AM, David Sterba wrote:
> (adding Harald to CC)
> 
> On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
>> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
>> This commit has the following problem:
>> 1) Break the ro mount rule.
>> When users mount the whole btrfs ro, it is still possible to mount
>> subvol rw and change the contents. Which make the whole fs ro mount
>> non-sense.
> 
> The proposed usecase was to allow mounting subvolumes with different
> ro/rw flags, and this makes sense to me (provided that the whole
> filesystem is mounted rw).
> 
> Anything else seems to lead to all the internal problems you point
> below. I'm not even sure if mounting the first subvolume 'ro' should
> imply that the whole filesystem is ro or not.
> 
>> 2) Cause whole btrfs ro/rw mount change fails.
>> When mount a subvol ro first, when you can't mount the whole fs mounted
>> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
>> which is OK for single fs to prevent mount fs ro in one mount point and
>> mount the same fs rw in other mount point.
>> Step to reproduce:
>> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
>> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail
> 
> Yeah, so first ro means whole filesystem is ro.
> 
>> 3) Kernel warn in vfs.
>> When mount the whole fs ro, and mount a subvol ro, kernel warning will
>> show in fs/sync.c complaining s_umount rwsem is not locked.
>> Since this remount is not called by VFS, so s_mounts rwsem is not
>> correctly locked.
> 
> That's serious.

Agreed, we'll pull this out until we get a better handle on things.
Thanks for spending time on it.

-chris

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Harald Hoyer]

On 01.07.2014 18:36, Chris Mason wrote:
> On 07/01/2014 11:32 AM, David Sterba wrote:
>> (adding Harald to CC)
>>
>> On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
>>> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
>>> This commit has the following problem:
>>> 1) Break the ro mount rule.
>>> When users mount the whole btrfs ro, it is still possible to mount
>>> subvol rw and change the contents. Which make the whole fs ro mount
>>> non-sense.
>>
>> The proposed usecase was to allow mounting subvolumes with different
>> ro/rw flags, and this makes sense to me (provided that the whole
>> filesystem is mounted rw).
>>
>> Anything else seems to lead to all the internal problems you point
>> below. I'm not even sure if mounting the first subvolume 'ro' should
>> imply that the whole filesystem is ro or not.
>>
>>> 2) Cause whole btrfs ro/rw mount change fails.
>>> When mount a subvol ro first, when you can't mount the whole fs mounted
>>> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
>>> which is OK for single fs to prevent mount fs ro in one mount point and
>>> mount the same fs rw in other mount point.
>>> Step to reproduce:
>>> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
>>> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail
>>
>> Yeah, so first ro means whole filesystem is ro.
>>
>>> 3) Kernel warn in vfs.
>>> When mount the whole fs ro, and mount a subvol ro, kernel warning will
>>> show in fs/sync.c complaining s_umount rwsem is not locked.
>>> Since this remount is not called by VFS, so s_mounts rwsem is not
>>> correctly locked.
>>
>> That's serious.
> 
> Agreed, we'll pull this out until we get a better handle on things.
> Thanks for spending time on it.
> 
> -chris
> 

My patch was initially only a request for comments:
- patch pointed out the problem
- provided a possible solution/workaround
- even has "FIXME" in the code :)

So, what I was hoping, that somebody else with more VFS knowledge than me would
step up and come up with a sane solution.

Pull it out, if the patch causes problems, but _please_ think about the problem
and come up with a solution, so that "mount -a" works with a normal fstab.

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Duncan]

Harald Hoyer posted on Wed, 02 Jul 2014 09:59:15 +0200 as excerpted:

> Pull it out, if the patch causes problems, but _please_ think about the
> problem and come up with a solution, so that "mount -a" works with a
> normal fstab.

FWIW, systemd makes it a LOT easier to manage multi-stage mounts with 
fstab automounts because with its mount-dependencies, in most cases it 
"just works".

My previous solution, gentoo so sysvinit with openrc mounting, required 
multiple mount stage scripts and painstaking dependency editing.  It 
worked, but it was a pain!

Finding out it pretty much "just worked" on systemd (tho I did have to 
manually add a single mount-dependency line to a single service unit, due 
to a cross-mount-point symlink that systemd didn't consider, but 
documentation was easy to find and the line did exactly what the 
documentation said it would do) was my most pleasant surprise of the 
switch! =:^)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes 
with different ro/rw options"
From: Harald Hoyer <harald@redhat.com>
To: Chris Mason <clm@fb.com>, dsterba@suse.cz, Qu Wenruo 
<quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Date: 2014年07月02日 15:59
> On 01.07.2014 18:36, Chris Mason wrote:
>> On 07/01/2014 11:32 AM, David Sterba wrote:
>>> (adding Harald to CC)
>>>
>>> On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
>>>> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
>>>> This commit has the following problem:
>>>> 1) Break the ro mount rule.
>>>> When users mount the whole btrfs ro, it is still possible to mount
>>>> subvol rw and change the contents. Which make the whole fs ro mount
>>>> non-sense.
>>> The proposed usecase was to allow mounting subvolumes with different
>>> ro/rw flags, and this makes sense to me (provided that the whole
>>> filesystem is mounted rw).
>>>
>>> Anything else seems to lead to all the internal problems you point
>>> below. I'm not even sure if mounting the first subvolume 'ro' should
>>> imply that the whole filesystem is ro or not.
>>>
>>>> 2) Cause whole btrfs ro/rw mount change fails.
>>>> When mount a subvol ro first, when you can't mount the whole fs mounted
>>>> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
>>>> which is OK for single fs to prevent mount fs ro in one mount point and
>>>> mount the same fs rw in other mount point.
>>>> Step to reproduce:
>>>> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
>>>> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail
>>> Yeah, so first ro means whole filesystem is ro.
>>>
>>>> 3) Kernel warn in vfs.
>>>> When mount the whole fs ro, and mount a subvol ro, kernel warning will
>>>> show in fs/sync.c complaining s_umount rwsem is not locked.
>>>> Since this remount is not called by VFS, so s_mounts rwsem is not
>>>> correctly locked.
>>> That's serious.
>> Agreed, we'll pull this out until we get a better handle on things.
>> Thanks for spending time on it.
>>
>> -chris
>>
> My patch was initially only a request for comments:
> - patch pointed out the problem
> - provided a possible solution/workaround
> - even has "FIXME" in the code :)
>
> So, what I was hoping, that somebody else with more VFS knowledge than me would
> step up and come up with a sane solution.
>
> Pull it out, if the patch causes problems, but _please_ think about the problem
> and come up with a solution, so that "mount -a" works with a normal fstab.
Personally, the main points to implement the feature are the following:
1) ro/rw logical (how ro and rw works)
IMO, any subvol can't be mounted rw if any of its parent is mounted ro 
would be good enough.
So whole btrfs ro mount will not allow any rw subvol mount,
and rw subvol will allow child subvol mounted as ro.

For me, the logical thing is not a problem.

2) subvol umount (real hard part to implement)
If we choose 1) logical, then we must store the rw/ro mount option will 
the subvolid when mount
and clear rw/ro mount option when *umounting the subvol*.

Mounting part can be easily done in fs/btrfs/super.c:get_default_root(), 
but the problem is that
when umount, we have no hook or callback to imform btrfs the subvol 
umount( I hope I was wrong).

It would be quite nice if someone has some good idea about this problem.

Thanks,
Qu

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Goffredo Baroncelli]

On 07/01/2014 11:30 AM, Qu Wenruo wrote:
> This commit has the following problem:
> 1) Break the ro mount rule.
> When users mount the whole btrfs ro, it is still possible to mount
> subvol rw and change the contents. Which make the whole fs ro mount
> non-sense.

Where is the problem ? I see an use case when I want a conservative default: mount all ro except some subvolumes.

In any case it is not a security problem because if the user has the capability to mount a subvolume, also he has the capability to remount,rw the whole filesystem.



-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes 
with different ro/rw options"
From: Goffredo Baroncelli <kreijack@libero.it>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Date: 2014年07月03日 01:48
> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>> This commit has the following problem:
>> 1) Break the ro mount rule.
>> When users mount the whole btrfs ro, it is still possible to mount
>> subvol rw and change the contents. Which make the whole fs ro mount
>> non-sense.
> Where is the problem ? I see an use case when I want a conservative default: mount all ro except some subvolumes.
>
> In any case it is not a security problem because if the user has the capability to mount a subvolume, also he has the capability to remount,rw the whole filesystem.
>
>
>
Not security problem but behavior not consistent.
If user mount the whole disk ro, he or she want the fs read only and 
nothing will change in it.
If you mount a subvol rw, then the whole disk ro expectation is broken. 
Things will change even the whole
disk is readonly.

The problem also happens when a parent subvol is mounted rw but child 
subvol is mounted ro.
User can still modify the child subvol through parent subvol, still 
broke the readonly rule.

Thanks,
Qu

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Chris Mason]

On 7/2/14, 3:59 AM, "Harald Hoyer" <harald@redhat.com> wrote:

>On 01.07.2014 18:36, Chris Mason wrote:
>> On 07/01/2014 11:32 AM, David Sterba wrote:
>>> (adding Harald to CC)
>>>
>>> On Tue, Jul 01, 2014 at 05:30:01PM +0800, Qu Wenruo wrote:
>>>> This reverts commit 0723a0473fb48a1c93b113a28665b64ce5faf35a.
>>>> This commit has the following problem:
>>>> 1) Break the ro mount rule.
>>>> When users mount the whole btrfs ro, it is still possible to mount
>>>> subvol rw and change the contents. Which make the whole fs ro mount
>>>> non-sense.
>>>
>>> The proposed usecase was to allow mounting subvolumes with different
>>> ro/rw flags, and this makes sense to me (provided that the whole
>>> filesystem is mounted rw).
>>>
>>> Anything else seems to lead to all the internal problems you point
>>> below. I'm not even sure if mounting the first subvolume 'ro' should
>>> imply that the whole filesystem is ro or not.
>>>
>>>> 2) Cause whole btrfs ro/rw mount change fails.
>>>> When mount a subvol ro first, when you can't mount the whole fs
>>>>mounted
>>>> rw. This is due to the check in btrfs_mount() which returns -EBUSY,
>>>> which is OK for single fs to prevent mount fs ro in one mount point
>>>>and
>>>> mount the same fs rw in other mount point.
>>>> Step to reproduce:
>>>> mount -o subvol=subv,ro /dev/sda6 /mnt/btrfs
>>>> mount -o rw /dev/sda6 /mnt/btrfs	<-this will fail
>>>
>>> Yeah, so first ro means whole filesystem is ro.
>>>
>>>> 3) Kernel warn in vfs.
>>>> When mount the whole fs ro, and mount a subvol ro, kernel warning will
>>>> show in fs/sync.c complaining s_umount rwsem is not locked.
>>>> Since this remount is not called by VFS, so s_mounts rwsem is not
>>>> correctly locked.
>>>
>>> That's serious.
>> 
>> Agreed, we'll pull this out until we get a better handle on things.
>> Thanks for spending time on it.
>> 
>> -chris
>> 
>
>My patch was initially only a request for comments:
>- patch pointed out the problem
>- provided a possible solution/workaround
>- even has "FIXME" in the code :)
>
>So, what I was hoping, that somebody else with more VFS knowledge than me
>would
>step up and come up with a sane solution.
>
>Pull it out, if the patch causes problems, but _please_ think about the
>problem
>and come up with a solution, so that "mount -a" works with a normal fstab.


Definitely, we don¹t want to drop this functionality over the long term.
The biggest problem here is the vfs warning.  Let me take another look and
see if I can get the locking right.

-chris

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Tobias Geerinckx-Rice]

[List CCd. I hate Gmail.]

Noob alert.

On 3 July 2014 02:28, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes w=
ith
> different ro/rw options"
> From: Goffredo Baroncelli <kreijack@libero.it>
> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
> Date: 2014=E5=B9=B407=E6=9C=8803=E6=97=A5 01:48
>>
>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>>
>>> This commit has the following problem:
>>> 1) Break the ro mount rule.
>>> When users mount the whole btrfs ro, it is still possible to mount
>>> subvol rw and change the contents. Which make the whole fs ro mount
>>> non-sense.
>>
>> Where is the problem ? I see an use case when I want a conservative
>> default: mount all ro except some subvolumes.
>>
>> In any case it is not a security problem because if the user has the
>> capability to mount a subvolume, also he has the capability to remount,r=
w
>> the whole filesystem.
>>
>>
>>
> Not security problem but behavior not consistent.
> If user mount the whole disk ro, he or she want the fs read only and noth=
ing
> will change in it.
> If you mount a subvol rw, then the whole disk ro expectation is broken.
> Things will change even the whole
> disk is readonly.

This assumption seems wrong and untenable if considered from a
different angle: one doesn't mount the "whole disk" ro, merely the
default subvolume.

# mount -o ro /dev/sda1 /mnt

is merely convenient short-hand for

# mount -o ro,subvol=3D@ [or whatever] /dev/sda1 /mnt

and anyone who expects this to magically protect the whole disk is,
frankly, confused.

Substituting partitions for subvolumes: mounting /dev/sda2 read-only
should have no effect on /dev/sda3.
Even if you went a bit batty and decided to make /dev/sda2 the
"default partition":

# ln -sf /dev/sda2 /dev/sda
# mount -o ro /dev/sda /mnt/this/is/silly

syntactic sugar doesn't change anything.

Subvolumes are logically discrete entities, the fact that they share
trees on-disk is merely a (very nice) implementation detail. It is
impossible to mount a "whole disk" under btrfs.

Tobias

> The problem also happens when a parent subvol is mounted rw but child sub=
vol
> is mounted ro.
> User can still modify the child subvol through parent subvol, still broke
> the readonly rule.

This makes sense, though.

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes 
with different ro/rw options"
From: Tobias Geerinckx-Rice <tobias.geerinckx.rice@gmail.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年07月03日 16:06
> [List CCd. I hate Gmail.]
>
> Noob alert.
>
> On 3 July 2014 02:28, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>> Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes w=
> ith
>> different ro/rw options"
>> From: Goffredo Baroncelli <kreijack@libero.it>
>> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
>> Date: 2014=E5=B9=B407=E6=9C=8803=E6=97=A5 01:48
>>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>>> This commit has the following problem:
>>>> 1) Break the ro mount rule.
>>>> When users mount the whole btrfs ro, it is still possible to mount
>>>> subvol rw and change the contents. Which make the whole fs ro mount
>>>> non-sense.
>>> Where is the problem ? I see an use case when I want a conservative
>>> default: mount all ro except some subvolumes.
>>>
>>> In any case it is not a security problem because if the user has the
>>> capability to mount a subvolume, also he has the capability to remount,r=
> w
>>> the whole filesystem.
>>>
>>>
>>>
>> Not security problem but behavior not consistent.
>> If user mount the whole disk ro, he or she want the fs read only and noth=
> ing
>> will change in it.
>> If you mount a subvol rw, then the whole disk ro expectation is broken.
>> Things will change even the whole
>> disk is readonly.
> This assumption seems wrong and untenable if considered from a
> different angle: one doesn't mount the "whole disk" ro, merely the
> default subvolume.
>
> # mount -o ro /dev/sda1 /mnt
>
> is merely convenient short-hand for
>
> # mount -o ro,subvol=3D@ [or whatever] /dev/sda1 /mnt
>
> and anyone who expects this to magically protect the whole disk is,
> frankly, confused.
>
> Substituting partitions for subvolumes: mounting /dev/sda2 read-only
> should have no effect on /dev/sda3.
> Even if you went a bit batty and decided to make /dev/sda2 the
> "default partition":
>
> # ln -sf /dev/sda2 /dev/sda
> # mount -o ro /dev/sda /mnt/this/is/silly
>
> syntactic sugar doesn't change anything.
>
> Subvolumes are logically discrete entities, the fact that they share
> trees on-disk is merely a (very nice) implementation detail. It is
> impossible to mount a "whole disk" under btrfs.
Oh, sorry for my confusing words.
To make it clear, when mentioning 'the whole disk(or partition 
whatever)' I mean the FS_TREE.
(Of course not the default subvolume)

The problem is that, even you mount a subvolume ro, you can still change 
contents in the subvolume
through its rw parent subvolume.
And if a subvolume can still be modified, the ro mount lose it meaning.

So we need special rules to prevent such things.

Thanks,
Qu
>
> Tobias
>
>> The problem also happens when a parent subvol is mounted rw but child sub=
> vol
>> is mounted ro.
>> User can still modify the child subvol through parent subvol, still broke
>> the readonly rule.
> This makes sense, though.

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Tobias Geerinckx-Rice]

On 3 July 2014 10:33, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> Oh, sorry for my confusing words.

And I probably should have waited for my frustration with my mail
client/device/public transport to subside before panicking^Creplying.

I use a combination of ro & rw (not insanely nested) subvolumes on a
few pseudo-embedded home/office servers and would like to keep that
arrangement working if possible. I'm also aware that it doesn't
protect against all possible bugs.

> To make it clear, when mentioning 'the whole disk(or partition whatever)' I mean the FS_TREE.
> (Of course not the default subvolume)
>
> The problem is that, even you mount a subvolume ro, you can still change contents in the subvolume
> through its rw parent subvolume.
> And if a subvolume can still be modified, the ro mount lose it meaning.

That makes so much more sense than my original reading, which was
weird and wrong and implied strange subvol-5-only magic. Sorry!

> So we need special rules to prevent such things.

Not that it matters, but: agreed.

   Tobias

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Goffredo Baroncelli]

On 07/03/2014 02:28 AM, Qu Wenruo wrote:
> 
> -------- Original Message --------
> Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"
> From: Goffredo Baroncelli <kreijack@libero.it>
> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
> Date: 2014年07月03日 01:48
>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>> This commit has the following problem:
>>> 1) Break the ro mount rule.
>>> When users mount the whole btrfs ro, it is still possible to mount
>>> subvol rw and change the contents. Which make the whole fs ro mount
>>> non-sense.
>> Where is the problem ? I see an use case when I want a conservative default: mount all ro except some subvolumes.
>>
>> In any case it is not a security problem because if the user has the capability to mount a subvolume, also he has the capability to remount,rw the whole filesystem.
>>
>>
>>
> Not security problem but behavior not consistent.
> If user mount the whole disk ro, he or she want the fs read only and nothing will change in it.
> If you mount a subvol rw, then the whole disk ro expectation is broken. Things will change even the whole
> disk is readonly.

Sorry for bother you again, but there is a thing not clear to me:

If

    # mount -o subvolid=5,ro /dev/sda1 /mnt/root
    # mount -o subvol=subvolname,rw /dev/sda1 /mnt/subvolname

I suppose that 

    # touch /mnt/root/touch-test 		# 1

fails, and

    # touch /mnt/subvolname/touch-test		# 2

succeeded. I understood correctly ? If so this behaviour seems to me correctly.
Different is after mounting the subvolume "subvolumename", also the whole filesystem results rw (eg: #1 succeeded).

G.Baroncelli





> 
> The problem also happens when a parent subvol is mounted rw but child subvol is mounted ro.
> User can still modify the child subvol through parent subvol, still broke the readonly rule.
> 
> Thanks,
> Qu
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes 
with different ro/rw options"
From: Goffredo Baroncelli <kreijack@inwind.it>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Date: 2014年07月04日 01:37
> On 07/03/2014 02:28 AM, Qu Wenruo wrote:
>> -------- Original Message --------
>> Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"
>> From: Goffredo Baroncelli <kreijack@libero.it>
>> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
>> Date: 2014年07月03日 01:48
>>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>>> This commit has the following problem:
>>>> 1) Break the ro mount rule.
>>>> When users mount the whole btrfs ro, it is still possible to mount
>>>> subvol rw and change the contents. Which make the whole fs ro mount
>>>> non-sense.
>>> Where is the problem ? I see an use case when I want a conservative default: mount all ro except some subvolumes.
>>>
>>> In any case it is not a security problem because if the user has the capability to mount a subvolume, also he has the capability to remount,rw the whole filesystem.
>>>
>>>
>>>
>> Not security problem but behavior not consistent.
>> If user mount the whole disk ro, he or she want the fs read only and nothing will change in it.
>> If you mount a subvol rw, then the whole disk ro expectation is broken. Things will change even the whole
>> disk is readonly.
> Sorry for bother you again, but there is a thing not clear to me:
>
> If
>
>      # mount -o subvolid=5,ro /dev/sda1 /mnt/root
>      # mount -o subvol=subvolname,rw /dev/sda1 /mnt/subvolname
>
> I suppose that
>
>      # touch /mnt/root/touch-test 		# 1
>
> fails, and
>
>      # touch /mnt/subvolname/touch-test		# 2
>
> succeeded. I understood correctly ?
Your understanding is right and that is current behavior.

But that should not be the correct behavior.

If you mount fs_tree ro, btrfs should ensure the whole fs_tree(including 
all the subvolumes) ro.
Or the whole fs_tree is not restricted readonly since you can modify 
contents inside the rw subvolume,
and it's part of the fs_tree.(partly ro and partly rw status)

IMO the perfect logical should be like the following:
1) ro mounted subvolume will force all the children subvolumes only ro 
mountable

subvol 5 (mounted ro /)
├── subvol 257 (mounted rw /mnt/btrfrs)
So above mounted should not be allowed.

But the following mount should be OK:
subvol 5 (mounted rw /)
├── subvol 257 (mounted ro /mnt/btrfrs)

2) ro mounted subvolume will not be modified even through the rw mounted 
parent subvolume.

Only this will ensure restricted ro mount option.

If anyone has any other ideas about it, I'm happy to listen.

Thanks,
Qu
>   If so this behaviour seems to me correctly.
> Different is after mounting the subvolume "subvolumename", also the whole filesystem results rw (eg: #1 succeeded).
>
> G.Baroncelli
>
>
>
>
>
>> The problem also happens when a parent subvol is mounted rw but child subvol is mounted ro.
>> User can still modify the child subvol through parent subvol, still broke the readonly rule.
>>
>> Thanks,
>> Qu
>>
>

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Goffredo Baroncelli]

Hi Qu
On 07/04/2014 03:28 AM, Qu Wenruo wrote:
> 
> -------- Original Message -------- Subject: Re: [RFC PATCH] Revert
> "btrfs: allow mounting btrfs subvolumes with different ro/rw
> options" From: Goffredo Baroncelli <kreijack@inwind.it> To: Qu Wenruo
> <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org Date: 2014年07月
> 04日 01:37
>> On 07/03/2014 02:28 AM, Qu Wenruo wrote:
>>> -------- Original Message -------- Subject: Re: [RFC PATCH]
>>> Revert "btrfs: allow mounting btrfs subvolumes with different
>>> ro/rw options" From: Goffredo Baroncelli <kreijack@libero.it> To:
>>> Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org 
>>> Date: 2014年07月03日 01:48
>>>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>>>> This commit has the following problem: 1) Break the ro mount
>>>>> rule. When users mount the whole btrfs ro, it is still
>>>>> possible to mount subvol rw and change the contents. Which
>>>>> make the whole fs ro mount non-sense.
>>>> Where is the problem ? I see an use case when I want a
>>>> conservative default: mount all ro except some subvolumes.
>>>> 
>>>> In any case it is not a security problem because if the user
>>>> has the capability to mount a subvolume, also he has the
>>>> capability to remount,rw the whole filesystem.
>>>> 
>>>> 
>>>> 
>>> Not security problem but behavior not consistent. If user mount
>>> the whole disk ro, he or she want the fs read only and nothing
>>> will change in it. If you mount a subvol rw, then the whole disk
>>> ro expectation is broken. Things will change even the whole disk
>>> is readonly.
>> Sorry for bother you again, but there is a thing not clear to me:
>> 
>> If
>> 
>> # mount -o subvolid=5,ro /dev/sda1 /mnt/root # mount -o
>> subvol=subvolname,rw /dev/sda1 /mnt/subvolname
>> 
>> I suppose that
>> 
>> # touch /mnt/root/touch-test         # 1
>> 
>> fails, and
>> 
>> # touch /mnt/subvolname/touch-test        # 2
>> 
>> succeeded. I understood correctly ?
> Your understanding is right and that is current behavior.
> 
> But that should not be the correct behavior.
> 
> If you mount fs_tree ro, btrfs should ensure the whole
> fs_tree(including all the subvolumes) ro. Or the whole fs_tree is not
> restricted readonly since you can modify contents inside the rw
> subvolume, and it's part of the fs_tree.(partly ro and partly rw
> status)
> 
> IMO the perfect logical should be like the following: 1) ro mounted
> subvolume will force all the children subvolumes only ro mountable
> 
> subvol 5 (mounted ro /) ├── subvol 257 (mounted rw /mnt/btrfrs) So
> above mounted should not be allowed.
> 
> But the following mount should be OK: subvol 5 (mounted rw /) ├──
> subvol 257 (mounted ro /mnt/btrfrs)
> 
> 2) ro mounted subvolume will not be modified even through the rw
> mounted parent subvolume.
> 
> Only this will ensure restricted ro mount option.
> 
> If anyone has any other ideas about it, I'm happy to listen.

Usually I mount both the subvolume as root filesystem and the root of the btrfs filesystem in a subdir . This allow me to perform action like snapshot of the root filesystem.

So to me it seems reasonable to have different rw/ro status between btrfs root and btrfs subvolume. As use case think a system which hosts several guests in container. Each guest has its own subvolume as root filesystem. An user would mount the btrfs root RO in order to see all the subvolume but at the same time he avoids to change a file for error; when a guest has to be started, its root filesystem/subvolume can be mounted RW.

On the other side, I understand that this could lead to an unexpected behaviour because with the other filesystem it is impossible to mount only a part as RW. In this BTRFS would be different.

Following the "least surprise" principle, I prefer that the *mount* RO/RW flag acts globally: the filesystem has only one status. It is possible to change it only globally.

In order to having a subvolumes with different RO/RW status we should rely on different flag. I have to point out that the subvolume has already the concept of read-only status.

We could adopt the following rules:
- if the filesystem is mounted RO then all the subvolumes (event the id=5) are RO
- if a subvolume is marked RO, the it is RO
- otherwise a subvolume is RW

Moreover we can add further rules to inherit the subvolume RO/RW status at the creation time (even tough it makes sense only for the snapshot). We could use an xattr for that.

Finally I would like to point out that relying on the relationship parent/child between the subvolumes is very dangerous. With the exception if the subvolid=5 which is the only root one, it is very easy to move up and down the subvolume. I have to point out this because I read on another email that someone likes the idea to having a RO subvolume because its parent is marked RO.
But a subvolume may be mounted also by id and not by its path (and or name). So relying on the relationship parent/child would lead to break the "least surprise principle".

My 2 ¢
BR
G.Baroncelli

> 
> Thanks, Qu
>> If so this behaviour seems to me correctly. Different is after
>> mounting the subvolume "subvolumename", also the whole filesystem
>> results rw (eg: #1 succeeded).
>> 
>> G.Baroncelli
>> 
>> 
>> 
>> 
>> 
>>> The problem also happens when a parent subvol is mounted rw but
>>> child subvol is mounted ro. User can still modify the child
>>> subvol through parent subvol, still broke the readonly rule.
>>> 
>>> Thanks, Qu
>>> 
>> 
> 
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes 
with different ro/rw options"
From: Goffredo Baroncelli <kreijack@inwind.it>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Date: 2014年07月05日 01:41
> Hi Qu
> On 07/04/2014 03:28 AM, Qu Wenruo wrote:
>> -------- Original Message -------- Subject: Re: [RFC PATCH] Revert
>> "btrfs: allow mounting btrfs subvolumes with different ro/rw
>> options" From: Goffredo Baroncelli <kreijack@inwind.it> To: Qu Wenruo
>> <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org Date: 2014年07月
>> 04日 01:37
>>> On 07/03/2014 02:28 AM, Qu Wenruo wrote:
>>>> -------- Original Message -------- Subject: Re: [RFC PATCH]
>>>> Revert "btrfs: allow mounting btrfs subvolumes with different
>>>> ro/rw options" From: Goffredo Baroncelli <kreijack@libero.it> To:
>>>> Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
>>>> Date: 2014年07月03日 01:48
>>>>> On 07/01/2014 11:30 AM, Qu Wenruo wrote:
>>>>>> This commit has the following problem: 1) Break the ro mount
>>>>>> rule. When users mount the whole btrfs ro, it is still
>>>>>> possible to mount subvol rw and change the contents. Which
>>>>>> make the whole fs ro mount non-sense.
>>>>> Where is the problem ? I see an use case when I want a
>>>>> conservative default: mount all ro except some subvolumes.
>>>>>
>>>>> In any case it is not a security problem because if the user
>>>>> has the capability to mount a subvolume, also he has the
>>>>> capability to remount,rw the whole filesystem.
>>>>>
>>>>>
>>>>>
>>>> Not security problem but behavior not consistent. If user mount
>>>> the whole disk ro, he or she want the fs read only and nothing
>>>> will change in it. If you mount a subvol rw, then the whole disk
>>>> ro expectation is broken. Things will change even the whole disk
>>>> is readonly.
>>> Sorry for bother you again, but there is a thing not clear to me:
>>>
>>> If
>>>
>>> # mount -o subvolid=5,ro /dev/sda1 /mnt/root # mount -o
>>> subvol=subvolname,rw /dev/sda1 /mnt/subvolname
>>>
>>> I suppose that
>>>
>>> # touch /mnt/root/touch-test         # 1
>>>
>>> fails, and
>>>
>>> # touch /mnt/subvolname/touch-test        # 2
>>>
>>> succeeded. I understood correctly ?
>> Your understanding is right and that is current behavior.
>>
>> But that should not be the correct behavior.
>>
>> If you mount fs_tree ro, btrfs should ensure the whole
>> fs_tree(including all the subvolumes) ro. Or the whole fs_tree is not
>> restricted readonly since you can modify contents inside the rw
>> subvolume, and it's part of the fs_tree.(partly ro and partly rw
>> status)
>>
>> IMO the perfect logical should be like the following: 1) ro mounted
>> subvolume will force all the children subvolumes only ro mountable
>>
>> subvol 5 (mounted ro /) ├── subvol 257 (mounted rw /mnt/btrfrs) So
>> above mounted should not be allowed.
>>
>> But the following mount should be OK: subvol 5 (mounted rw /) ├──
>> subvol 257 (mounted ro /mnt/btrfrs)
>>
>> 2) ro mounted subvolume will not be modified even through the rw
>> mounted parent subvolume.
>>
>> Only this will ensure restricted ro mount option.
>>
>> If anyone has any other ideas about it, I'm happy to listen.
> Usually I mount both the subvolume as root filesystem and the root of the btrfs filesystem in a subdir . This allow me to perform action like snapshot of the root filesystem.
>
> So to me it seems reasonable to have different rw/ro status between btrfs root and btrfs subvolume. As use case think a system which hosts several guests in container. Each guest has its own subvolume as root filesystem. An user would mount the btrfs root RO in order to see all the subvolume but at the same time he avoids to change a file for error; when a guest has to be started, its root filesystem/subvolume can be mounted RW.
You caught me. Yes, the use case seems quite resonable since currently 
you need to mount btrfs to get the subvolume list
(the only offline method seems btrfs-debug-tree but end-users won't use 
it anyway)
and it's a good admin behavior to mount it ro if no need to write.
>
> On the other side, I understand that this could lead to an unexpected behaviour because with the other filesystem it is impossible to mount only a part as RW. In this BTRFS would be different.
>
> Following the "least surprise" principle, I prefer that the *mount* RO/RW flag acts globally: the filesystem has only one status. It is possible to change it only globally.
>
> In order to having a subvolumes with different RO/RW status we should rely on different flag. I have to point out that the subvolume has already the concept of read-only status.
>
> We could adopt the following rules:
> - if the filesystem is mounted RO then all the subvolumes (event the id=5) are RO
> - if a subvolume is marked RO, the it is RO
> - otherwise a subvolume is RW
I'm confused with rule 1. When mentionting 'mounted RO', you mean mount 
subvolume id=5 RO?
Also you mentioned that using differnt RO/RW flag independent from VFS 
RO/RW flags,
so it also makes me confused that when mentioning RO, did you mean VFS 
RO or new btrfs RO/RW flags?
>
> Moreover we can add further rules to inherit the subvolume RO/RW status at the creation time (even tough it makes sense only for the snapshot). We could use an xattr for that.
>
> Finally I would like to point out that relying on the relationship parent/child between the subvolumes is very dangerous. With the exception if the subvolid=5 which is the only root one, it is very easy to move up and down the subvolume. I have to point out this because I read on another email that someone likes the idea to having a RO subvolume because its parent is marked RO.
> But a subvolume may be mounted also by id and not by its path (and or name). So relying on the relationship parent/child would lead to break the "least surprise principle".
>
> My 2 ¢
> BR
> G.Baroncelli
Oh I forgot that user can mv subvolumes just like normal dirs. In this 
case it will certainly make ro/rw disaster if rely on the parent ro/rw 
status. :(

Thanks,
Qu
>
>> Thanks, Qu
>>> If so this behaviour seems to me correctly. Different is after
>>> mounting the subvolume "subvolumename", also the whole filesystem
>>> results rw (eg: #1 succeeded).
>>>
>>> G.Baroncelli
>>>
>>>
>>>
>>>
>>>
>>>> The problem also happens when a parent subvol is mounted rw but
>>>> child subvol is mounted ro. User can still modify the child
>>>> subvol through parent subvol, still broke the readonly rule.
>>>>
>>>> Thanks, Qu
>>>>
>>
>

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Goffredo Baroncelli]

On 07/07/2014 03:46 AM, Qu Wenruo wrote:
> 
[... cut ...]
>> 
>> So to me it seems reasonable to have different rw/ro status between
>> btrfs root and btrfs subvolume. As use case think a system which
>> hosts several guests in container. Each guest has its own subvolume
>> as root filesystem. An user would mount the btrfs root RO in order
>> to see all the subvolume but at the same time he avoids to change a
>> file for error; when a guest has to be started, its root
>> filesystem/subvolume can be mounted RW.
> You caught me. Yes, the use case seems quite resonable since
> currently you need to mount btrfs to get the subvolume list (the only
> offline method seems btrfs-debug-tree but end-users won't use it
> anyway) and it's a good admin behavior to mount it ro if no need to
> write.
>> 
>> On the other side, I understand that this could lead to an
>> unexpected behaviour because with the other filesystem it is
>> impossible to mount only a part as RW. In this BTRFS would be
>> different.
>> 
>> Following the "least surprise" principle, I prefer that the *mount*
>> RO/RW flag acts globally: the filesystem has only one status. It is
>> possible to change it only globally.
>> 
>> In order to having a subvolumes with different RO/RW status we
>> should rely on different flag. I have to point out that the
>> subvolume has already the concept of read-only status.
>> 
>> We could adopt the following rules: 

>> - if the filesystem is mounted RO then all the subvolumes (event 
>> the id=5) are RO 


>>- if a subvolume is marked RO, the it is RO 
>> - otherwise a subvolume is RW

> I'm confused with rule 1. When mentionting 'mounted RO', you mean
> mount subvolume id=5 RO? Also you mentioned that using differnt RO/RW
> flag independent from VFS RO/RW flags, so it also makes me confused
> that when mentioning RO, did you mean VFS RO or new btrfs RO/RW
> flags?


For "mounted RO" I mean the VFS flag, the "one" passed via the mount 
command. I say "one" as 1, because I am convinced that it has to act globally, 
e.g. on the whole filesystem; the flag should be set at the first mount,
then it can be changed (only ?) issuing a "mount -o remount,rw/ro"

For example, the following commands

  # mount -o subvol=subvolname,ro /dev/sda1 /mnt/btrfs-subvol
  # mount -o subvolid=5 /dev/sda1 /mnt/btrfs-root

cause the following ones

  # touch /mnt/btrfs-subvol/touch-a-file
  # touch /mnt/btrfs-root/touch-a-file2

to fail; and the following commands

  # mount -o subvol=subvolname,ro /dev/sda1 /mnt/btrfs-subvol
  # mount -o subvolid=5 /dev/sda1 /mnt/btrfs-root
  # mount -o remount,rw /mnt/btrfs-subvol

cause the following ones 

  # touch /mnt/btrfs-subvol/touch-a-file
  # touch /mnt/btrfs-root/touch-a-file2

to succeed

So for each filesystem, there is a "globally" flag ro/rw which acts on the 
whole filesystem. Clear and simple.

Step 2: a more fine grained control of the subvolumes.
We have already the capability to make a subvolume read-only/read-write doing

   # btrfs property set -t s /path/to/subvolume ro true

or 

   # btrfs property set -t s /path/to/subvolume ro false

My idea is to use this flag. It could be done at the mount time for example:

  # mount -o subvolmode=ro,subvol=subvolname /dev/sda1 /

(this example doesn't work, it is only a my idea)

So:
- we should not add further code
- the semantic is simple
- the property is linked to the subvolume in a understandable way

We should only add the subvolmode=ro option to the mount command.

Further discussion need to investigate the following cases:
- if the filesystem is mounted as ro (mount -o ro....), does mounting a subvolume
 rw ( mount -o subvolmode=rw...) should raise an error ?  (IMHO yes) 
- if the filesystem is mounted as ro (mount -o ro....), does mounting the filesystem a 2nd time rw ( mount -o rw...) should raise an error ?  (IMHO yes) 
- if a subvolume is mounter rw (or ro), does mounting the same subvolume a 2nd time as ro (or rw) should raise an error ?  (IMHO yes) 


BR
G.Baroncelli

>> 
>> Moreover we can add further rules to inherit the subvolume RO/RW
>> status at the creation time (even tough it makes sense only for the
>> snapshot). We could use an xattr for that.
>> 
>> Finally I would like to point out that relying on the relationship
>> parent/child between the subvolumes is very dangerous. With the
>> exception if the subvolid=5 which is the only root one, it is very
>> easy to move up and down the subvolume. I have to point out this
>> because I read on another email that someone likes the idea to
>> having a RO subvolume because its parent is marked RO. But a
>> subvolume may be mounted also by id and not by its path (and or
>> name). So relying on the relationship parent/child would lead to
>> break the "least surprise principle".
>> 
>> My 2 ¢ BR G.Baroncelli
> Oh I forgot that user can mv subvolumes just like normal dirs. In
> this case it will certainly make ro/rw disaster if rely on the parent
> ro/rw status. :(
> 
> Thanks, Qu
>> 
>>> Thanks, Qu
>>>> If so this behaviour seems to me correctly. Different is after 
>>>> mounting the subvolume "subvolumename", also the whole
>>>> filesystem results rw (eg: #1 succeeded).
>>>> 
>>>> G.Baroncelli
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>>> The problem also happens when a parent subvol is mounted rw
>>>>> but child subvol is mounted ro. User can still modify the
>>>>> child subvol through parent subvol, still broke the readonly
>>>>> rule.
>>>>> 
>>>>> Thanks, Qu
>>>>> 
>>> 
>> 
> 
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Duncan]

Goffredo Baroncelli posted on Mon, 07 Jul 2014 19:37:53 +0200 as
excerpted:

> For "mounted RO" I mean the VFS flag, the "one" passed via the mount
> command. I say "one" as 1, because I am convinced that it has to act
> globally,
> e.g. on the whole filesystem; the flag should be set at the first mount,
> then it can be changed (only ?) issuing a "mount -o remount,rw/ro"
[...]
> So for each filesystem, there is a "globally" flag ro/rw which acts on
> the whole filesystem. Clear and simple.
> 
> Step 2: a more fine grained control of the subvolumes.
> We have already the capability to make a subvolume read-only/read-write
> doing
> 
>    # btrfs property set -t s /path/to/subvolume ro true
> 
> or
> 
>    # btrfs property set -t s /path/to/subvolume ro false
> 
> My idea is to use this flag. It could be done at the mount time for
> example:
> 
>   # mount -o subvolmode=ro,subvol=subvolname /dev/sda1 /
> 
> (this example doesn't work, it is only a my idea)
> 
> So:
> - we should not add further code
> - the semantic is simple
> - the property is linked to the subvolume in a understandable way
> 
> We should only add the subvolmode=ro option to the mount command.
> 
> Further discussion need to investigate the following cases:
> - if the filesystem is mounted as ro (mount -o ro....), does mounting a
> subvolume rw ( mount -o subvolmode=rw...) should raise an error ? 
> (IMHO yes)
> - if the filesystem is mounted as ro (mount -o ro....), does mounting
> the filesystem a 2nd time rw ( mount -o rw...) should raise an error ? 
> (IMHO yes)
> - if a subvolume is mounter rw (or ro), does mounting the same subvolume
> a 2nd time as ro (or rw) should raise an error ?
> (IMHO yes)

Makes sense.

Assuming I'm following you correctly, then, no subvolumes could be rw if 
the filesystem/vfs flag was set ro.

Which would mean that in ordered to mount any particular subvolume rw, 
the whole filesystem would have to be rw.

Extending now:

For simplicity and backward compatibility, if subvolmode isn't set, it 
corresponds to the whole-fs/vfs mode.  That way, setting mount -o ro,... 
(or -o rw,...) with the first mount would naturally propagate to all 
subsequent subvolume mounts, unless of course (1) all subvolumes and the 
filesystem itself are umounted, after which a new mount would be the 
first one again, or (2) a mount -o remount,... is done that changes the 
whole-fs mode.

Further, if it happened that one wanted the first subvolume mounted to be 
ro, but the filesystem as a whole rw so that other subvolumes could be 
mounted rw, the following would accomplish that:

mount -o rw,subvolmode=ro

That way, the subvol would be ro as desired, but the filesystem as a 
whole would be rw, so other subvolumes could be successfully mounted rw.

I like the concept. =:^)

The remaining problem to deal with is that if say the root subvol (id=5) 
is mounted rw,subvolmode=rw, while a subvolume below it is mounted 
subvolmode=ro, then what happens if someone tries to make an edit in the 
portion of the filesystem visible in the subvolume, but from the parent, 
id=5/root in this case?  Obviously if that modification is allowed from 
the parent, it'll change what's visible in the child subvolume as well, 
which would be rather unexpected.

I'd suggest that the snapshotting border rule should apply to writes as 
well.  Snapshots stop at subvolume borders, and writes should as well.  
Attempting to write in a child subvolume should error out -- child 
subvolumes are not part of a parent snapshot and shouldn't be writable 
from the parent subvolume, either.  Child-subvolume content should be 
read-only because it's beyond the subvolume border.

That would seem to be the safest.  Altho I believe it's a change from 
current behavior, where it's possible to write into any subvolume visible 
from the parent (that is, not covered by an over-mount, perhaps even of 
the same subvolume that would otherwise be visible in the same location 
from the parent subvolume), provided the parent is writable.

Regardless, my biggest take-away from the discussion so far is that I'm 
glad I decided to go with entirely separate filesystems, each on their 
own partitions, so my ro vs writable mounts do exactly what I expect them 
to do without me having to worry or think about it too much!  That wasn't 
the reason I did it -- I did it because I didn't want all my data eggs in 
the same whole-filesystem basket such that if a filesystem was damaged, 
the damage was compartmentalized -- but now that see all the subvolume rw/
ro implications discussed in this thread, I'm VERY glad I personally 
don't have to worry about it, and it all simply "just works" for me, 
because each filesystem is independent of the others, not simply a 
subvolume!

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [RFC PATCH] Revert "btrfs: allow mounting btrfs subvolumes with different ro/rw options"

[Goffredo Baroncelli]

On 07/08/2014 04:43 AM, Duncan wrote:
> The remaining problem to deal with is that if say the root subvol (id=5) 
> is mounted rw,subvolmode=rw, while a subvolume below it is mounted 
> subvolmode=ro, then what happens if someone tries to make an edit in the 
> portion of the filesystem visible in the subvolume, but from the parent, 
> id=5/root in this case?  Obviously if that modification is allowed from 
> the parent, it'll change what's visible in the child subvolume as well, 
> which would be rather unexpected.

The ro/rw status is a subvolume flag.
So if a subvolume is marked rw (or ro), is writable (not writable) in all
the mount(S)
This flag is not inheritable.

What could be strange is the following:

      # mount -o subvolid=5,rw /dev/sda1 /mnt/btrfs-root
      # btrfs subvol create /mnt/btrfs-root/subvolname/
then
      # touch /mnt/btrfs-root/subvolname/touch-file

succeeds; but

      # mount -o subvolid=5,rw /dev/sda1 /mnt/btrfs-root
      # btrfs subvol create /mnt/btrfs-root/subvolname/
      # mount -o subvol=subvolname,ro /dev/sda1 /mnt/btrfs-subvol
then
      # touch /mnt/btrfs-root/subvolname/touch-file2

fails.




-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5