[RFC] Improve subvolume usability for a normal user

[RFC] Improve subvolume usability for a normal user

[Misono, Tomohiro]

Hello all,

I want to address some issues of subvolume usability for a normal user.
i.e. a user can create subvolumes, but
 - Cannot delete their own subvolume (by default)
 - Cannot tell subvolumes from directories (in a straightforward way)
 - Cannot check the quota limit when qgroup is enabled

Here I show the initial thoughts and approaches to this problem.
I want to check if this is a right approach or not before I start writing code.

Comments are welcome.
Tomohiro Misono

==========
- Goal and current problem
The goal of this RFC is to give a normal user more control to their own subvolumes.
Currently the control to subvolumes for a normal user is restricted as below: 

+-------------+------+------+
|   command   | root | user |
+-------------+------+------+
| sub create  | Y    | Y    |
| sub snap    | Y    | Y    |
| sub del     | Y    | N    |
| sub list    | Y    | N    |
| sub show    | Y    | N    |
| qgroup show | Y    | N    |
+-------------+------+------+

In short, I want to change this as below in order to improve user's usability:

+-------------+------+--------+
|   command   | root | user   |
+-------------+------+--------+
| sub create  | Y    | Y      |
| sub snap    | Y    | Y      |
| sub del     | Y    | N -> Y |
| sub list    | Y    | N -> Y |
| sub show    | Y    | N -> Y |
| qgroup show | Y    | N -> Y |
+-------------+------+--------+

In words,
(1) allow deletion of subvolume if a user owns it, and 
(2) allow getting subvolume/quota info if a user has read access to it
 (sub list/qgroup show just lists the subvolumes which are readable by the user)

I think other commands not listed above (qgroup limit, send/receive etc.) should
be done by root and not be allowed for a normal user.


- Outside the scope of this RFC
There is a qualitative problem to use qgroup for limiting user disk amount;
quota limit can easily be averted by creating a subvolume. I think that forcing
inheriting quota of parent subvolume is a solution, but I won't address nor
discuss this here. 


- Proposal
 (1) deletion of subvolume

  I want to change the default behavior to allow a user to delete their own
  subvolumes.
  
  This is not the same behavior as when user_subvol_rm_alowed mount option is
  specified; in that case a user can delete the subvolume to which they have
  write+exec right.
  
  Since snapshot creation is already restricted to the subvolume owner, it is
  consistent that only the owner of the subvolume (or root) can delete it.
  
  The implementation should be straightforward.

 (2) getting subvolume/quota info

  TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
  (sub show/list, qgroup show etc.). This requires the root permission.
  
  The easiest way to allow a user to get subvolume/quota info is just relaxing
  the permission of TREE_SEARCH. However, since all the tree information (inc.
  file name) will be exposed, this poses a sequrity risk and is not acceptable.
  
  So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
  a normal user, which return only the info readable by the user:
  
   [subvolume info]
    Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
   right by searching FS/FILE tree and comparing it with caller's uid.
  
   [quota info]
    Same as above, but mainly search QUOTA tree and only returns level 0 info.
  
  Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
  needs to be relaxed for the user who has read access to the subvolume.


- Summary of Proposal
 - Change the default behavior to allow a user to delete their own subvolumes
 - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes
==========

Re: [RFC] Improve subvolume usability for a normal user

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4803 bytes --]



On 2017年12月05日 16:25, Misono, Tomohiro wrote:
> Hello all,
> 
> I want to address some issues of subvolume usability for a normal user.
> i.e. a user can create subvolumes, but
>  - Cannot delete their own subvolume (by default)
>  - Cannot tell subvolumes from directories (in a straightforward way)
>  - Cannot check the quota limit when qgroup is enabled
> 
> Here I show the initial thoughts and approaches to this problem.
> I want to check if this is a right approach or not before I start writing code.
> 
> Comments are welcome.
> Tomohiro Misono
> 
> ==========
> - Goal and current problem
> The goal of this RFC is to give a normal user more control to their own subvolumes.
> Currently the control to subvolumes for a normal user is restricted as below: 
> 
> +-------------+------+------+
> |   command   | root | user |
> +-------------+------+------+
> | sub create  | Y    | Y    |
> | sub snap    | Y    | Y    |
> | sub del     | Y    | N    |
> | sub list    | Y    | N    |
> | sub show    | Y    | N    |
> | qgroup show | Y    | N    |
> +-------------+------+------+
> 
> In short, I want to change this as below in order to improve user's usability:
> 
> +-------------+------+--------+
> |   command   | root | user   |
> +-------------+------+--------+
> | sub create  | Y    | Y      |
> | sub snap    | Y    | Y      |
> | sub del     | Y    | N -> Y |
> | sub list    | Y    | N -> Y |
> | sub show    | Y    | N -> Y |
> | qgroup show | Y    | N -> Y |
> +-------------+------+--------+
> 
> In words,
> (1) allow deletion of subvolume if a user owns it, and 
> (2) allow getting subvolume/quota info if a user has read access to it
>  (sub list/qgroup show just lists the subvolumes which are readable by the user)
> 
> I think other commands not listed above (qgroup limit, send/receive etc.) should
> be done by root and not be allowed for a normal user.
> 
> 
> - Outside the scope of this RFC
> There is a qualitative problem to use qgroup for limiting user disk amount;
> quota limit can easily be averted by creating a subvolume. I think that forcing
> inheriting quota of parent subvolume is a solution, but I won't address nor
> discuss this here. 
> 
> 
> - Proposal
>  (1) deletion of subvolume
> 
>   I want to change the default behavior to allow a user to delete their own
>   subvolumes.
>   
>   This is not the same behavior as when user_subvol_rm_alowed mount option is
>   specified; in that case a user can delete the subvolume to which they have
>   write+exec right.
>   
>   Since snapshot creation is already restricted to the subvolume owner, it is
>   consistent that only the owner of the subvolume (or root) can delete it.
>   
>   The implementation should be straightforward.

Personally speaking, I prefer to do the complex owner check in user daemon.

And do the privilege in user daemon (call it btrfsd for example).

So btrfs-progs will works in 2 modes, if root calls it, do as it used to do.
If normal user calls it, proxy the request to btrfsd, and btrfsd does
the privilege checking and call ioctl (with root privilege).

Then no impact to kernel, all complex work is done in user space.

> 
>  (2) getting subvolume/quota info
> 
>   TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
>   (sub show/list, qgroup show etc.). This requires the root permission.
>   
>   The easiest way to allow a user to get subvolume/quota info is just relaxing
>   the permission of TREE_SEARCH. However, since all the tree information (inc.
>   file name) will be exposed, this poses a sequrity risk and is not acceptable.
>   
>   So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
>   a normal user, which return only the info readable by the user:
>   
>    [subvolume info]
>     Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
>    right by searching FS/FILE tree and comparing it with caller's uid.
>   
>    [quota info]
>     Same as above, but mainly search QUOTA tree and only returns level 0 info.
>   
>   Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
>   needs to be relaxed for the user who has read access to the subvolume.

Same method can apply to this either.

Thanks,
Qu
> 
> 
> - Summary of Proposal
>  - Change the default behavior to allow a user to delete their own subvolumes
>  - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes
> ==========
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [RFC] Improve subvolume usability for a normal user

[Austin S. Hemmelgarn]

On 2017-12-05 03:43, Qu Wenruo wrote:
> 
> 
> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>> Hello all,
>>
>> I want to address some issues of subvolume usability for a normal user.
>> i.e. a user can create subvolumes, but
>>   - Cannot delete their own subvolume (by default)
>>   - Cannot tell subvolumes from directories (in a straightforward way)
>>   - Cannot check the quota limit when qgroup is enabled
>>
>> Here I show the initial thoughts and approaches to this problem.
>> I want to check if this is a right approach or not before I start writing code.
>>
>> Comments are welcome.
>> Tomohiro Misono
>>
>> ==========
>> - Goal and current problem
>> The goal of this RFC is to give a normal user more control to their own subvolumes.
>> Currently the control to subvolumes for a normal user is restricted as below:
>>
>> +-------------+------+------+
>> |   command   | root | user |
>> +-------------+------+------+
>> | sub create  | Y    | Y    |
>> | sub snap    | Y    | Y    |
>> | sub del     | Y    | N    |
>> | sub list    | Y    | N    |
>> | sub show    | Y    | N    |
>> | qgroup show | Y    | N    |
>> +-------------+------+------+
>>
>> In short, I want to change this as below in order to improve user's usability:
>>
>> +-------------+------+--------+
>> |   command   | root | user   |
>> +-------------+------+--------+
>> | sub create  | Y    | Y      |
>> | sub snap    | Y    | Y      |
>> | sub del     | Y    | N -> Y |
>> | sub list    | Y    | N -> Y |
>> | sub show    | Y    | N -> Y |
>> | qgroup show | Y    | N -> Y |
>> +-------------+------+--------+
>>
>> In words,
>> (1) allow deletion of subvolume if a user owns it, and
>> (2) allow getting subvolume/quota info if a user has read access to it
>>   (sub list/qgroup show just lists the subvolumes which are readable by the user)
>>
>> I think other commands not listed above (qgroup limit, send/receive etc.) should
>> be done by root and not be allowed for a normal user.
>>
>>
>> - Outside the scope of this RFC
>> There is a qualitative problem to use qgroup for limiting user disk amount;
>> quota limit can easily be averted by creating a subvolume. I think that forcing
>> inheriting quota of parent subvolume is a solution, but I won't address nor
>> discuss this here.
>>
>>
>> - Proposal
>>   (1) deletion of subvolume
>>
>>    I want to change the default behavior to allow a user to delete their own
>>    subvolumes.
>>    
>>    This is not the same behavior as when user_subvol_rm_alowed mount option is
>>    specified; in that case a user can delete the subvolume to which they have
>>    write+exec right.
>>    
>>    Since snapshot creation is already restricted to the subvolume owner, it is
>>    consistent that only the owner of the subvolume (or root) can delete it.
>>    
>>    The implementation should be straightforward.
> 
> Personally speaking, I prefer to do the complex owner check in user daemon.
> 
> And do the privilege in user daemon (call it btrfsd for example).
> 
> So btrfs-progs will works in 2 modes, if root calls it, do as it used to do.
> If normal user calls it, proxy the request to btrfsd, and btrfsd does
> the privilege checking and call ioctl (with root privilege).
> 
> Then no impact to kernel, all complex work is done in user space.
Exactly how hard is it to just check ownership of the root inode of a 
subvolume from the ioctl context?  You could just as easily push all the 
checking out to the VFS layer by taking an open fd for the subvolume 
root (and probably implicitly closing it) instead of taking a path, and 
that would give you all the benefits of ACL's and whatever security 
modules the local system is using.  Alternatively, quit treating 
subvolumes specially for `unlink()`, and make them behave like regular 
directories (and thus avoid the one possible complication resulting from 
home directories being subvolumes but needing to be owned by the users).

<rant>
This whole 'just make it a daemon' thing that's becoming the standard 
solution everywhere these days is crap in most cases (note that I'm not 
just singling out this proposal here).  Just because systemd does it 
doesn't make it a good idea, especially when it makes things like this 
far more opaque than they really need to be, which in turn makes it a 
pain in the arse to debug _anything_ to do with permissions beyond basic 
file access.
</rant>

In almost all cases, the desired check is to allow root, possibly admin 
users who have filesystem management permissions (usually exactly one 
group), and the owner.  That is trivial to implement with a single ACL 
entry and basic filesystem permissions without needing some special 
daemon to check them provided we route things through the VFS layer 
somehow.  In the rare cases that that's not what's wanted, then the 
system is almost certainly using an LSM, and the checks can be made there.
> 
>>
>>   (2) getting subvolume/quota info
>>
>>    TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
>>    (sub show/list, qgroup show etc.). This requires the root permission.
>>    
>>    The easiest way to allow a user to get subvolume/quota info is just relaxing
>>    the permission of TREE_SEARCH. However, since all the tree information (inc.
>>    file name) will be exposed, this poses a sequrity risk and is not acceptable.
>>    
>>    So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
>>    a normal user, which return only the info readable by the user:
>>    
>>     [subvolume info]
>>      Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
>>     right by searching FS/FILE tree and comparing it with caller's uid.
>>    
>>     [quota info]
>>      Same as above, but mainly search QUOTA tree and only returns level 0 info.
>>    
>>    Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
>>    needs to be relaxed for the user who has read access to the subvolume.
> 
> Same method can apply to this either.
> 
> Thanks,
> Qu
>>
>>
>> - Summary of Proposal
>>   - Change the default behavior to allow a user to delete their own subvolumes
>>   - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes

Re: [RFC] Improve subvolume usability for a normal user

[Graham Cobb]

On 05/12/17 12:41, Austin S. Hemmelgarn wrote:
> On 2017-12-05 03:43, Qu Wenruo wrote:
>>
>>
>> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>>> Hello all,
>>>
>>> I want to address some issues of subvolume usability for a normal user.
>>> i.e. a user can create subvolumes, but
>>>   - Cannot delete their own subvolume (by default)
>>>   - Cannot tell subvolumes from directories (in a straightforward way)
>>>   - Cannot check the quota limit when qgroup is enabled
>>>
>>> Here I show the initial thoughts and approaches to this problem.
>>> I want to check if this is a right approach or not before I start
>>> writing code.
>>>
>>> Comments are welcome.
>>> Tomohiro Misono
>>>
>>> ==========
>>> - Goal and current problem
>>> The goal of this RFC is to give a normal user more control to their
>>> own subvolumes.
>>> Currently the control to subvolumes for a normal user is restricted
>>> as below:
>>>
>>> +-------------+------+------+
>>> |   command   | root | user |
>>> +-------------+------+------+
>>> | sub create  | Y    | Y    |
>>> | sub snap    | Y    | Y    |
>>> | sub del     | Y    | N    |
>>> | sub list    | Y    | N    |
>>> | sub show    | Y    | N    |
>>> | qgroup show | Y    | N    |
>>> +-------------+------+------+
>>>
>>> In short, I want to change this as below in order to improve user's
>>> usability:
>>>
>>> +-------------+------+--------+
>>> |   command   | root | user   |
>>> +-------------+------+--------+
>>> | sub create  | Y    | Y      |
>>> | sub snap    | Y    | Y      |
>>> | sub del     | Y    | N -> Y |
>>> | sub list    | Y    | N -> Y |
>>> | sub show    | Y    | N -> Y |
>>> | qgroup show | Y    | N -> Y |
>>> +-------------+------+--------+
>>>
>>> In words,
>>> (1) allow deletion of subvolume if a user owns it, and
>>> (2) allow getting subvolume/quota info if a user has read access to it
>>>   (sub list/qgroup show just lists the subvolumes which are readable
>>> by the user)
>>>
>>> I think other commands not listed above (qgroup limit, send/receive
>>> etc.) should
>>> be done by root and not be allowed for a normal user.
>>>
>>>
>>> - Outside the scope of this RFC
>>> There is a qualitative problem to use qgroup for limiting user disk
>>> amount;
>>> quota limit can easily be averted by creating a subvolume. I think
>>> that forcing
>>> inheriting quota of parent subvolume is a solution, but I won't
>>> address nor
>>> discuss this here.
>>>
>>>
>>> - Proposal
>>>   (1) deletion of subvolume
>>>
>>>    I want to change the default behavior to allow a user to delete
>>> their own
>>>    subvolumes.
>>>       This is not the same behavior as when user_subvol_rm_alowed
>>> mount option is
>>>    specified; in that case a user can delete the subvolume to which
>>> they have
>>>    write+exec right.
>>>       Since snapshot creation is already restricted to the subvolume
>>> owner, it is
>>>    consistent that only the owner of the subvolume (or root) can
>>> delete it.
>>>       The implementation should be straightforward.
>>
>> Personally speaking, I prefer to do the complex owner check in user
>> daemon.
>>
>> And do the privilege in user daemon (call it btrfsd for example).
>>
>> So btrfs-progs will works in 2 modes, if root calls it, do as it used
>> to do.
>> If normal user calls it, proxy the request to btrfsd, and btrfsd does
>> the privilege checking and call ioctl (with root privilege).
>>
>> Then no impact to kernel, all complex work is done in user space.
> Exactly how hard is it to just check ownership of the root inode of a
> subvolume from the ioctl context?  You could just as easily push all the
> checking out to the VFS layer by taking an open fd for the subvolume
> root (and probably implicitly closing it) instead of taking a path, and
> that would give you all the benefits of ACL's and whatever security
> modules the local system is using.  

+1 - stop inventing new access control rules for each different action!

Re: [RFC] Improve subvolume usability for a normal user

[Goffredo Baroncelli]

On 12/05/2017 04:42 PM, Graham Cobb wrote:
> On 05/12/17 12:41, Austin S. Hemmelgarn wrote:
>> On 2017-12-05 03:43, Qu Wenruo wrote:
>>>
>>>
>>> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>>>> Hello all,
>>>>
>>>> I want to address some issues of subvolume usability for a normal user.
>>>> i.e. a user can create subvolumes, but
>>>>   - Cannot delete their own subvolume (by default)
>>>>   - Cannot tell subvolumes from directories (in a straightforward way)
>>>>   - Cannot check the quota limit when qgroup is enabled
>>>>
>>>> Here I show the initial thoughts and approaches to this problem.
>>>> I want to check if this is a right approach or not before I start
>>>> writing code.
>>>>
>>>> Comments are welcome.
>>>> Tomohiro Misono
>>>>
>>>> ==========
>>>> - Goal and current problem
>>>> The goal of this RFC is to give a normal user more control to their
>>>> own subvolumes.
>>>> Currently the control to subvolumes for a normal user is restricted
>>>> as below:
>>>>
>>>> +-------------+------+------+
>>>> |   command   | root | user |
>>>> +-------------+------+------+
>>>> | sub create  | Y    | Y    |
>>>> | sub snap    | Y    | Y    |
>>>> | sub del     | Y    | N    |
>>>> | sub list    | Y    | N    |
>>>> | sub show    | Y    | N    |
>>>> | qgroup show | Y    | N    |
>>>> +-------------+------+------+
>>>>
>>>> In short, I want to change this as below in order to improve user's
>>>> usability:
>>>>
>>>> +-------------+------+--------+
>>>> |   command   | root | user   |
>>>> +-------------+------+--------+
>>>> | sub create  | Y    | Y      |
>>>> | sub snap    | Y    | Y      |
>>>> | sub del     | Y    | N -> Y |
>>>> | sub list    | Y    | N -> Y |
>>>> | sub show    | Y    | N -> Y |
>>>> | qgroup show | Y    | N -> Y |
>>>> +-------------+------+--------+
>>>>
>>>> In words,
>>>> (1) allow deletion of subvolume if a user owns it, and
>>>> (2) allow getting subvolume/quota info if a user has read access to it
>>>>   (sub list/qgroup show just lists the subvolumes which are readable
>>>> by the user)
>>>>
>>>> I think other commands not listed above (qgroup limit, send/receive
>>>> etc.) should
>>>> be done by root and not be allowed for a normal user.
>>>>
>>>>
>>>> - Outside the scope of this RFC
>>>> There is a qualitative problem to use qgroup for limiting user disk
>>>> amount;
>>>> quota limit can easily be averted by creating a subvolume. I think
>>>> that forcing
>>>> inheriting quota of parent subvolume is a solution, but I won't
>>>> address nor
>>>> discuss this here.
>>>>
>>>>
>>>> - Proposal
>>>>   (1) deletion of subvolume
>>>>
>>>>    I want to change the default behavior to allow a user to delete
>>>> their own
>>>>    subvolumes.
>>>>       This is not the same behavior as when user_subvol_rm_alowed
>>>> mount option is
>>>>    specified; in that case a user can delete the subvolume to which
>>>> they have
>>>>    write+exec right.
>>>>       Since snapshot creation is already restricted to the subvolume
>>>> owner, it is
>>>>    consistent that only the owner of the subvolume (or root) can
>>>> delete it.
>>>>       The implementation should be straightforward.
>>>
>>> Personally speaking, I prefer to do the complex owner check in user
>>> daemon.
>>>
>>> And do the privilege in user daemon (call it btrfsd for example).
>>>
>>> So btrfs-progs will works in 2 modes, if root calls it, do as it used
>>> to do.
>>> If normal user calls it, proxy the request to btrfsd, and btrfsd does
>>> the privilege checking and call ioctl (with root privilege).
>>>
>>> Then no impact to kernel, all complex work is done in user space.
>> Exactly how hard is it to just check ownership of the root inode of a
>> subvolume from the ioctl context?  You could just as easily push all the
>> checking out to the VFS layer by taking an open fd for the subvolume
>> root (and probably implicitly closing it) instead of taking a path, and
>> that would give you all the benefits of ACL's and whatever security
>> modules the local system is using.  
> 
> +1 - stop inventing new access control rules for each different action!

A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.

In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
a) the user is root  or
b.1) if the subvolume is empty and
b.2) the user has the write capability to the parent directory

This will solve all the problems:
a) removing a subvolume is like removing a directory; no different check is performed; no new (and strange) rule
b) an ordinary user can remove a subvolume, like a directory
c) is quite easy to implement

Root still have another way (more efficient) to remove a subvolume: he can invoke the BTRFS_IOC_SNAP_DESTROY, which doesn't need to traverse all the tree.

BR
G.Baroncelli


[1] https://www.spinics.net/lists/linux-btrfs/msg06499.html
[2] https://patchwork.kernel.org/patch/260301/
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

Re: [RFC] Improve subvolume usability for a normal user

[Graham Cobb]

On 05/12/17 18:01, Goffredo Baroncelli wrote:
> On 12/05/2017 04:42 PM, Graham Cobb wrote:
>> On 05/12/17 12:41, Austin S. Hemmelgarn wrote:
>>> On 2017-12-05 03:43, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>>>>> Hello all,
>>>>>
>>>>> I want to address some issues of subvolume usability for a normal user.
>>>>> i.e. a user can create subvolumes, but
>>>>>   - Cannot delete their own subvolume (by default)
>>>>>   - Cannot tell subvolumes from directories (in a straightforward way)
>>>>>   - Cannot check the quota limit when qgroup is enabled
>>>>>
>>>>> Here I show the initial thoughts and approaches to this problem.
>>>>> I want to check if this is a right approach or not before I start
>>>>> writing code.
>>>>>
>>>>> Comments are welcome.
>>>>> Tomohiro Misono
>>>>>
>>>>> ==========
>>>>> - Goal and current problem
>>>>> The goal of this RFC is to give a normal user more control to their
>>>>> own subvolumes.
>>>>> Currently the control to subvolumes for a normal user is restricted
>>>>> as below:
>>>>>
>>>>> +-------------+------+------+
>>>>> |   command   | root | user |
>>>>> +-------------+------+------+
>>>>> | sub create  | Y    | Y    |
>>>>> | sub snap    | Y    | Y    |
>>>>> | sub del     | Y    | N    |
>>>>> | sub list    | Y    | N    |
>>>>> | sub show    | Y    | N    |
>>>>> | qgroup show | Y    | N    |
>>>>> +-------------+------+------+
>>>>>
>>>>> In short, I want to change this as below in order to improve user's
>>>>> usability:
>>>>>
>>>>> +-------------+------+--------+
>>>>> |   command   | root | user   |
>>>>> +-------------+------+--------+
>>>>> | sub create  | Y    | Y      |
>>>>> | sub snap    | Y    | Y      |
>>>>> | sub del     | Y    | N -> Y |
>>>>> | sub list    | Y    | N -> Y |
>>>>> | sub show    | Y    | N -> Y |
>>>>> | qgroup show | Y    | N -> Y |
>>>>> +-------------+------+--------+
>>>>>
>>>>> In words,
>>>>> (1) allow deletion of subvolume if a user owns it, and
>>>>> (2) allow getting subvolume/quota info if a user has read access to it
>>>>>   (sub list/qgroup show just lists the subvolumes which are readable
>>>>> by the user)
>>>>>
>>>>> I think other commands not listed above (qgroup limit, send/receive
>>>>> etc.) should
>>>>> be done by root and not be allowed for a normal user.
>>>>>
>>>>>
>>>>> - Outside the scope of this RFC
>>>>> There is a qualitative problem to use qgroup for limiting user disk
>>>>> amount;
>>>>> quota limit can easily be averted by creating a subvolume. I think
>>>>> that forcing
>>>>> inheriting quota of parent subvolume is a solution, but I won't
>>>>> address nor
>>>>> discuss this here.
>>>>>
>>>>>
>>>>> - Proposal
>>>>>   (1) deletion of subvolume
>>>>>
>>>>>    I want to change the default behavior to allow a user to delete
>>>>> their own
>>>>>    subvolumes.
>>>>>       This is not the same behavior as when user_subvol_rm_alowed
>>>>> mount option is
>>>>>    specified; in that case a user can delete the subvolume to which
>>>>> they have
>>>>>    write+exec right.
>>>>>       Since snapshot creation is already restricted to the subvolume
>>>>> owner, it is
>>>>>    consistent that only the owner of the subvolume (or root) can
>>>>> delete it.
>>>>>       The implementation should be straightforward.
>>>>
>>>> Personally speaking, I prefer to do the complex owner check in user
>>>> daemon.
>>>>
>>>> And do the privilege in user daemon (call it btrfsd for example).
>>>>
>>>> So btrfs-progs will works in 2 modes, if root calls it, do as it used
>>>> to do.
>>>> If normal user calls it, proxy the request to btrfsd, and btrfsd does
>>>> the privilege checking and call ioctl (with root privilege).
>>>>
>>>> Then no impact to kernel, all complex work is done in user space.
>>> Exactly how hard is it to just check ownership of the root inode of a
>>> subvolume from the ioctl context?  You could just as easily push all the
>>> checking out to the VFS layer by taking an open fd for the subvolume
>>> root (and probably implicitly closing it) instead of taking a path, and
>>> that would give you all the benefits of ACL's and whatever security
>>> modules the local system is using.  
>>
>> +1 - stop inventing new access control rules for each different action!
> 
> A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
> This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.
> 
> In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
> a) the user is root  or
> b.1) if the subvolume is empty and
> b.2) the user has the write capability to the parent directory

That is also OK, as it follows a logical and consistent model without
introducing new rules, but it has two disadvantages with snapshots the
user creates and then wants to delete:

i) they have to change the snapshot to readwrite to remove all the
contents -- how many users will know that that can even be done?

ii) if the snapshot contains a file the user cannot remove then the user
cannot remove the snapshot at all (even though they could create it).

That is why I preferred Austin's first proposal. I suppose your proposal
could be extended:

a) the user is root  or
b.1) if the subvolume is empty and
b.2) the user has the write capability to the parent directory, or
c) the subvolume is readonly

However it is done, I think it is important that all normal VFS access
rules (such as ACLs) are followed for the subvolume itself.

Re: [RFC] Improve subvolume usability for a normal user

[Goffredo Baroncelli]

On 12/05/2017 07:46 PM, Graham Cobb wrote:
> On 05/12/17 18:01, Goffredo Baroncelli wrote:
>> On 12/05/2017 04:42 PM, Graham Cobb wrote:
[....]
>>>>> Then no impact to kernel, all complex work is done in user space.
>>>> Exactly how hard is it to just check ownership of the root inode of a
>>>> subvolume from the ioctl context?  You could just as easily push all the
>>>> checking out to the VFS layer by taking an open fd for the subvolume
>>>> root (and probably implicitly closing it) instead of taking a path, and
>>>> that would give you all the benefits of ACL's and whatever security
>>>> modules the local system is using.  
>>>
>>> +1 - stop inventing new access control rules for each different action!
>>
>> A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
>> This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.
>>
>> In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
>> a) the user is root  or
>> b.1) if the subvolume is empty and
>> b.2) the user has the write capability to the parent directory
> 
> That is also OK, as it follows a logical and consistent model without
> introducing new rules, but it has two disadvantages with snapshots the
> user creates and then wants to delete:
> 
> i) they have to change the snapshot to readwrite to remove all the
> contents -- how many users will know that that can even be done?

I think that this is an orthogonal question.  However the user should use btrfs set....

Anyway I am not against to use something more specific like "btrfs sub del...", where it could be possible to implement a more specific checks. I am only highlight that destroy a subvolume without looking to its content could break the POSIX rules

> 
> ii) if the snapshot contains a file the user cannot remove then the user
> cannot remove the snapshot at all (even though they could create it).

It is the same for the other filesystem: if in a filesystem tree, there is a directory which is not changeable by the user, the user cannot remove all the tree. Again, I am only highlighting that there is a possible break of POSIX rules.

> 
> That is why I preferred Austin's first proposal. I suppose your proposal
> could be extended:
> 
> a) the user is root  or
> b.1) if the subvolume is empty and
> b.2) the user has the write capability to the parent directory, or
> c) the subvolume is readonly

If a subvolume is marked read-only, the user has to change to RW via "btrfs property set...."

> 
> However it is done, I think it is important that all normal VFS access
> rules (such as ACLs) are followed for the subvolume itself.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

Re: [RFC] Improve subvolume usability for a normal user

[Austin S. Hemmelgarn]

On 2017-12-05 14:09, Goffredo Baroncelli wrote:
> On 12/05/2017 07:46 PM, Graham Cobb wrote:
>> On 05/12/17 18:01, Goffredo Baroncelli wrote:
>>> On 12/05/2017 04:42 PM, Graham Cobb wrote:
> [....]
>>>>>> Then no impact to kernel, all complex work is done in user space.
>>>>> Exactly how hard is it to just check ownership of the root inode of a
>>>>> subvolume from the ioctl context?  You could just as easily push all the
>>>>> checking out to the VFS layer by taking an open fd for the subvolume
>>>>> root (and probably implicitly closing it) instead of taking a path, and
>>>>> that would give you all the benefits of ACL's and whatever security
>>>>> modules the local system is using.
>>>>
>>>> +1 - stop inventing new access control rules for each different action!
>>>
>>> A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
>>> This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.
>>>
>>> In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
>>> a) the user is root  or
>>> b.1) if the subvolume is empty and
>>> b.2) the user has the write capability to the parent directory
>>
>> That is also OK, as it follows a logical and consistent model without
>> introducing new rules, but it has two disadvantages with snapshots the
>> user creates and then wants to delete:
>>
>> i) they have to change the snapshot to readwrite to remove all the
>> contents -- how many users will know that that can even be done?
> 
> I think that this is an orthogonal question.  However the user should use btrfs set....
> 
> Anyway I am not against to use something more specific like "btrfs sub del...", where it could be possible to implement a more specific checks. I am only highlight that destroy a subvolume without looking to its content could break the POSIX rules
No, it doesn't break POSIX rules, because you can't do it with POSIX 
defined interfaces (not counting ioctls, but ioctls are by definition 
system dependent).
> 
>>
>> ii) if the snapshot contains a file the user cannot remove then the user
>> cannot remove the snapshot at all (even though they could create it).
> 
> It is the same for the other filesystem: if in a filesystem tree, there is a directory which is not changeable by the user, the user cannot remove all the tree. Again, I am only highlighting that there is a possible break of POSIX rules.
POSIX says nothing about non-permissions related reasons for not being 
able to remove a directory.   A subvolume is treated (too much in my 
opinion) like a mount point, regardless of whether or not it's 
explicitly mounted, and that seems to be most of why you can't remove it 
with unlink().
> 
>>
>> That is why I preferred Austin's first proposal. I suppose your proposal
>> could be extended:
>>
>> a) the user is root  or
>> b.1) if the subvolume is empty and
>> b.2) the user has the write capability to the parent directory, or
>> c) the subvolume is readonly
> 
> If a subvolume is marked read-only, the user has to change to RW via "btrfs property set...."
I actually do agree with this assessment to a certain extent, but the 
user has to be able to change the subvolume to be RW to begin with (and 
that _does_ need an in-kernel permissions check beyond just knowing 
whether you can access the subvolume).

Re: [RFC] Improve subvolume usability for a normal user

[Goffredo Baroncelli]

On 12/05/2017 09:17 PM, Austin S. Hemmelgarn wrote:
> On 2017-12-05 14:09, Goffredo Baroncelli wrote:
>> On 12/05/2017 07:46 PM, Graham Cobb wrote:
>>> On 05/12/17 18:01, Goffredo Baroncelli wrote:
>>>> On 12/05/2017 04:42 PM, Graham Cobb wrote:
>> [....]
>>>>>>> Then no impact to kernel, all complex work is done in user space.
>>>>>> Exactly how hard is it to just check ownership of the root inode of a
>>>>>> subvolume from the ioctl context?  You could just as easily push all the
>>>>>> checking out to the VFS layer by taking an open fd for the subvolume
>>>>>> root (and probably implicitly closing it) instead of taking a path, and
>>>>>> that would give you all the benefits of ACL's and whatever security
>>>>>> modules the local system is using.
>>>>>
>>>>> +1 - stop inventing new access control rules for each different action!
>>>>
>>>> A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
>>>> This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.
>>>>
>>>> In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
>>>> a) the user is root  or
>>>> b.1) if the subvolume is empty and
>>>> b.2) the user has the write capability to the parent directory
>>>
>>> That is also OK, as it follows a logical and consistent model without
>>> introducing new rules, but it has two disadvantages with snapshots the
>>> user creates and then wants to delete:
>>>
>>> i) they have to change the snapshot to readwrite to remove all the
>>> contents -- how many users will know that that can even be done?
>>
>> I think that this is an orthogonal question.  However the user should use btrfs set....
>>
>> Anyway I am not against to use something more specific like "btrfs sub del...", where it could be possible to implement a more specific checks. I am only highlight that destroy a subvolume without looking to its content could break the POSIX rules
> No, it doesn't break POSIX rules, because you can't do it with POSIX defined interfaces (not counting ioctls, but ioctls are by definition system dependent).

user$ mkdir -p dir1/dir2
user$ touch dir1/dir2/file
user$ sudo chown foo:foo dir1/dir2

user$ rm -rf dir1/
rm: cannot remove 'dir1/dir2/file1': Permission denied

In a POSIX filesystem, you (as user) cannot remove file1.

But if instead of dir1 you have a subvolume (called sub1), and the check of removing the subvolume is performed only on the root of subvolume, you could remove it.

Is it a problem ? I think no, but I am not a security expert. But even if POSIX doesn't have the concept of subvolume, this is definitely a break of a POSIX rule.

On the other side, if the user makes a snapshot of a subvolume containing a not owned and not empty directory, for the user it would be impossible to delete its snapshot (!).


>>
>>>
>>> ii) if the snapshot contains a file the user cannot remove then the user
>>> cannot remove the snapshot at all (even though they could create it).
>>
>> It is the same for the other filesystem: if in a filesystem tree, there is a directory which is not changeable by the user, the user cannot remove all the tree. Again, I am only highlighting that there is a possible break of POSIX rules.
> POSIX says nothing about non-permissions related reasons for not being able to remove a directory. 
Nobody has say the opposite

>  A subvolume is treated (too much in my opinion) like a mount point, regardless of whether or not it's explicitly mounted, and that seems to be most of why you can't remove it with unlink().



>>
>>>
>>> That is why I preferred Austin's first proposal. I suppose your proposal
>>> could be extended:
>>>
>>> a) the user is root  or
>>> b.1) if the subvolume is empty and
>>> b.2) the user has the write capability to the parent directory, or
>>> c) the subvolume is readonly
>>
>> If a subvolume is marked read-only, the user has to change to RW via "btrfs property set...."
> I actually do agree with this assessment to a certain extent, but the user has to be able to change the subvolume to be RW to begin with (and that _does_ need an in-kernel permissions check beyond just knowing whether you can access the subvolume).
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

Re: [RFC] Improve subvolume usability for a normal user

[Austin S. Hemmelgarn]

On 2017-12-05 17:08, Goffredo Baroncelli wrote:
> On 12/05/2017 09:17 PM, Austin S. Hemmelgarn wrote:
>> On 2017-12-05 14:09, Goffredo Baroncelli wrote:
>>> On 12/05/2017 07:46 PM, Graham Cobb wrote:
>>>> On 05/12/17 18:01, Goffredo Baroncelli wrote:
>>>>> On 12/05/2017 04:42 PM, Graham Cobb wrote:
>>> [....]
>>>>>>>> Then no impact to kernel, all complex work is done in user space.
>>>>>>> Exactly how hard is it to just check ownership of the root inode of a
>>>>>>> subvolume from the ioctl context?  You could just as easily push all the
>>>>>>> checking out to the VFS layer by taking an open fd for the subvolume
>>>>>>> root (and probably implicitly closing it) instead of taking a path, and
>>>>>>> that would give you all the benefits of ACL's and whatever security
>>>>>>> modules the local system is using.
>>>>>>
>>>>>> +1 - stop inventing new access control rules for each different action!
>>>>>
>>>>> A subvolume is like a directory; In all filesystems you cannot remove an inode if you don't have write access to the parent directory. I assume that this is a POSIX requirement; and if so this should be true even for BTRFS.
>>>>> This means that in order to remove a subvolume and you are not root, you should check all the contained directories. It is not sufficient to check one inode.
>>>>>
>>>>> In the past I create a patch [1][2] which extended the unlink (2) syscall to allow removing a subvolume if:
>>>>> a) the user is root  or
>>>>> b.1) if the subvolume is empty and
>>>>> b.2) the user has the write capability to the parent directory
>>>>
>>>> That is also OK, as it follows a logical and consistent model without
>>>> introducing new rules, but it has two disadvantages with snapshots the
>>>> user creates and then wants to delete:
>>>>
>>>> i) they have to change the snapshot to readwrite to remove all the
>>>> contents -- how many users will know that that can even be done?
>>>
>>> I think that this is an orthogonal question.  However the user should use btrfs set....
>>>
>>> Anyway I am not against to use something more specific like "btrfs sub del...", where it could be possible to implement a more specific checks. I am only highlight that destroy a subvolume without looking to its content could break the POSIX rules
>> No, it doesn't break POSIX rules, because you can't do it with POSIX defined interfaces (not counting ioctls, but ioctls are by definition system dependent).
> 
> user$ mkdir -p dir1/dir2
> user$ touch dir1/dir2/file
> user$ sudo chown foo:foo dir1/dir2
> 
> user$ rm -rf dir1/
> rm: cannot remove 'dir1/dir2/file1': Permission denied
> 
> In a POSIX filesystem, you (as user) cannot remove file1.
> 
> But if instead of dir1 you have a subvolume (called sub1), and the check of removing the subvolume is performed only on the root of subvolume, you could remove it.
> 
> Is it a problem ? I think no, but I am not a security expert. But even if POSIX doesn't have the concept of subvolume, this is definitely a break of a POSIX rule.
Except it isn't a break of a POSIX rule, because deleting the subvolume 
doesn't go through regular POSIX API's (again, ioctl really doesn't 
count because it's designed for random extended stuff).  POSIX makes 
absolutely zero guarantees about the behavior of things not done through 
POSIX API's.  Yes, this doesn't use POSIX style permission checks, but 
it also doesn't violate POSIX standards either (because it's not defined 
in the POSIX standard, and things that aren't defined by the standards 
are explicitly stated to be implementation defined).

The data loss risk this entails is however the reason users can't delete 
subvolumes by default.
> 
> On the other side, if the user makes a snapshot of a subvolume containing a not owned and not empty directory, for the user it would be impossible to delete its snapshot (!).
I would argue that by default they shouldn't be able to make a snapshot 
because:
1. With the default behavior of users not being able to delete 
subvolumes, users can trivially cause a resource exhaustion situation 
that they can't resolve themself.
2. Whitelisting is usually better for security than blacklisting.
> 
> 
>>>
>>>>
>>>> ii) if the snapshot contains a file the user cannot remove then the user
>>>> cannot remove the snapshot at all (even though they could create it).
>>>
>>> It is the same for the other filesystem: if in a filesystem tree, there is a directory which is not changeable by the user, the user cannot remove all the tree. Again, I am only highlighting that there is a possible break of POSIX rules.
>> POSIX says nothing about non-permissions related reasons for not being able to remove a directory.
> Nobody has say the opposite
Claiming that it's potentially POSIX incompatible behavior directly 
implies that POSIX has provisions regarding non-permissions related 
reasons for not being able to remove a directory.
> 
>>    A subvolume is treated (too much in my opinion) like a mount point, regardless of whether or not it's explicitly mounted, and that seems to be most of why you can't remove it with unlink().
>>>
>>>>
>>>> That is why I preferred Austin's first proposal. I suppose your proposal
>>>> could be extended:
>>>>
>>>> a) the user is root  or
>>>> b.1) if the subvolume is empty and
>>>> b.2) the user has the write capability to the parent directory, or
>>>> c) the subvolume is readonly
>>>
>>> If a subvolume is marked read-only, the user has to change to RW via "btrfs property set...."
>> I actually do agree with this assessment to a certain extent, but the user has to be able to change the subvolume to be RW to begin with (and that _does_ need an in-kernel permissions check 
>> beyond just knowing whether you can access the subvolume).

Re: [RFC] Improve subvolume usability for a normal user

[Misono, Tomohiro]

On 2017/12/05 21:41, Austin S. Hemmelgarn wrote:
> On 2017-12-05 03:43, Qu Wenruo wrote:
>>
>>
>> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>>> Hello all,
>>>
>>> I want to address some issues of subvolume usability for a normal user.
>>> i.e. a user can create subvolumes, but
>>>   - Cannot delete their own subvolume (by default)
>>>   - Cannot tell subvolumes from directories (in a straightforward way)
>>>   - Cannot check the quota limit when qgroup is enabled
>>>
>>> Here I show the initial thoughts and approaches to this problem.
>>> I want to check if this is a right approach or not before I start writing code.
>>>
>>> Comments are welcome.
>>> Tomohiro Misono
>>>
>>> ==========
>>> - Goal and current problem
>>> The goal of this RFC is to give a normal user more control to their own subvolumes.
>>> Currently the control to subvolumes for a normal user is restricted as below:
>>>
>>> +-------------+------+------+
>>> |   command   | root | user |
>>> +-------------+------+------+
>>> | sub create  | Y    | Y    |
>>> | sub snap    | Y    | Y    |
>>> | sub del     | Y    | N    |
>>> | sub list    | Y    | N    |
>>> | sub show    | Y    | N    |
>>> | qgroup show | Y    | N    |
>>> +-------------+------+------+
>>>
>>> In short, I want to change this as below in order to improve user's usability:
>>>
>>> +-------------+------+--------+
>>> |   command   | root | user   |
>>> +-------------+------+--------+
>>> | sub create  | Y    | Y      |
>>> | sub snap    | Y    | Y      |
>>> | sub del     | Y    | N -> Y |
>>> | sub list    | Y    | N -> Y |
>>> | sub show    | Y    | N -> Y |
>>> | qgroup show | Y    | N -> Y |
>>> +-------------+------+--------+
>>>
>>> In words,
>>> (1) allow deletion of subvolume if a user owns it, and
>>> (2) allow getting subvolume/quota info if a user has read access to it
>>>   (sub list/qgroup show just lists the subvolumes which are readable by the user)
>>>
>>> I think other commands not listed above (qgroup limit, send/receive etc.) should
>>> be done by root and not be allowed for a normal user.
>>>
>>>
>>> - Outside the scope of this RFC
>>> There is a qualitative problem to use qgroup for limiting user disk amount;
>>> quota limit can easily be averted by creating a subvolume. I think that forcing
>>> inheriting quota of parent subvolume is a solution, but I won't address nor
>>> discuss this here.
>>>
>>>
>>> - Proposal
>>>   (1) deletion of subvolume
>>>
>>>    I want to change the default behavior to allow a user to delete their own
>>>    subvolumes.
>>>    
>>>    This is not the same behavior as when user_subvol_rm_alowed mount option is
>>>    specified; in that case a user can delete the subvolume to which they have
>>>    write+exec right.
>>>    
>>>    Since snapshot creation is already restricted to the subvolume owner, it is
>>>    consistent that only the owner of the subvolume (or root) can delete it.
>>>    
>>>    The implementation should be straightforward.
>>
>> Personally speaking, I prefer to do the complex owner check in user daemon.
>>
>> And do the privilege in user daemon (call it btrfsd for example).
>>
>> So btrfs-progs will works in 2 modes, if root calls it, do as it used to do.
>> If normal user calls it, proxy the request to btrfsd, and btrfsd does
>> the privilege checking and call ioctl (with root privilege).
>>
>> Then no impact to kernel, all complex work is done in user space.
> Exactly how hard is it to just check ownership of the root inode of a 
> subvolume from the ioctl context?  You could just as easily push all the 
> checking out to the VFS layer by taking an open fd for the subvolume 
> root (and probably implicitly closing it) instead of taking a path, and 
> that would give you all the benefits of ACL's and whatever security 
> modules the local system is using.  Alternatively, quit treating 
> subvolumes specially for `unlink()`, and make them behave like regular 
> directories (and thus avoid the one possible complication resulting from 
> home directories being subvolumes but needing to be owned by the users).

Thanks to all for the comments.
Let me explain my stance clearly:

I'm for limiting the subvolume operation to btrfs-progs commands since a
subvolume looks like a regular directory, but is different in many points.
This is the reason I want to fix not only "sub delete" but also "sub show/list" etc.

I think "sub delete" should be the counterpart of "sub create" and "sub snap". 
Since "sub snap" can be used for the subvolume the user owns, "sub delete" 
should be allowed to the suvolume the user owns too. Also, using these
commands enables an instant creation/deletion of a  subvolume/snapshot, which
is one of the features of btrfs.

Regards,
Tomohiro
> 
> <rant>
> This whole 'just make it a daemon' thing that's becoming the standard 
> solution everywhere these days is crap in most cases (note that I'm not 
> just singling out this proposal here).  Just because systemd does it 
> doesn't make it a good idea, especially when it makes things like this 
> far more opaque than they really need to be, which in turn makes it a 
> pain in the arse to debug _anything_ to do with permissions beyond basic 
> file access.
> </rant>
> 
> In almost all cases, the desired check is to allow root, possibly admin 
> users who have filesystem management permissions (usually exactly one 
> group), and the owner.  That is trivial to implement with a single ACL 
> entry and basic filesystem permissions without needing some special 
> daemon to check them provided we route things through the VFS layer 
> somehow.  In the rare cases that that's not what's wanted, then the 
> system is almost certainly using an LSM, and the checks can be made ther>>
>>>
>>>   (2) getting subvolume/quota info
>>>
>>>    TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
>>>    (sub show/list, qgroup show etc.). This requires the root permission.
>>>    
>>>    The easiest way to allow a user to get subvolume/quota info is just relaxing
>>>    the permission of TREE_SEARCH. However, since all the tree information (inc.
>>>    file name) will be exposed, this poses a sequrity risk and is not acceptable.
>>>    
>>>    So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
>>>    a normal user, which return only the info readable by the user:
>>>    
>>>     [subvolume info]
>>>      Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
>>>     right by searching FS/FILE tree and comparing it with caller's uid.
>>>    
>>>     [quota info]
>>>      Same as above, but mainly search QUOTA tree and only returns level 0 info.
>>>    
>>>    Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
>>>    needs to be relaxed for the user who has read access to the subvolume.
>>
>> Same method can apply to this either.
>>
>> Thanks,
>> Qu
>>>
>>>
>>> - Summary of Proposal
>>>   - Change the default behavior to allow a user to delete their own subvolumes
>>>   - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC] Improve subvolume usability for a normal user

[Austin S. Hemmelgarn]

On 2017-12-05 23:52, Misono, Tomohiro wrote:
> On 2017/12/05 21:41, Austin S. Hemmelgarn wrote:
>> On 2017-12-05 03:43, Qu Wenruo wrote:
>>>
>>>
>>> On 2017年12月05日 16:25, Misono, Tomohiro wrote:
>>>> Hello all,
>>>>
>>>> I want to address some issues of subvolume usability for a normal user.
>>>> i.e. a user can create subvolumes, but
>>>>    - Cannot delete their own subvolume (by default)
>>>>    - Cannot tell subvolumes from directories (in a straightforward way)
>>>>    - Cannot check the quota limit when qgroup is enabled
>>>>
>>>> Here I show the initial thoughts and approaches to this problem.
>>>> I want to check if this is a right approach or not before I start writing code.
>>>>
>>>> Comments are welcome.
>>>> Tomohiro Misono
>>>>
>>>> ==========
>>>> - Goal and current problem
>>>> The goal of this RFC is to give a normal user more control to their own subvolumes.
>>>> Currently the control to subvolumes for a normal user is restricted as below:
>>>>
>>>> +-------------+------+------+
>>>> |   command   | root | user |
>>>> +-------------+------+------+
>>>> | sub create  | Y    | Y    |
>>>> | sub snap    | Y    | Y    |
>>>> | sub del     | Y    | N    |
>>>> | sub list    | Y    | N    |
>>>> | sub show    | Y    | N    |
>>>> | qgroup show | Y    | N    |
>>>> +-------------+------+------+
>>>>
>>>> In short, I want to change this as below in order to improve user's usability:
>>>>
>>>> +-------------+------+--------+
>>>> |   command   | root | user   |
>>>> +-------------+------+--------+
>>>> | sub create  | Y    | Y      |
>>>> | sub snap    | Y    | Y      |
>>>> | sub del     | Y    | N -> Y |
>>>> | sub list    | Y    | N -> Y |
>>>> | sub show    | Y    | N -> Y |
>>>> | qgroup show | Y    | N -> Y |
>>>> +-------------+------+--------+
>>>>
>>>> In words,
>>>> (1) allow deletion of subvolume if a user owns it, and
>>>> (2) allow getting subvolume/quota info if a user has read access to it
>>>>    (sub list/qgroup show just lists the subvolumes which are readable by the user)
>>>>
>>>> I think other commands not listed above (qgroup limit, send/receive etc.) should
>>>> be done by root and not be allowed for a normal user.
>>>>
>>>>
>>>> - Outside the scope of this RFC
>>>> There is a qualitative problem to use qgroup for limiting user disk amount;
>>>> quota limit can easily be averted by creating a subvolume. I think that forcing
>>>> inheriting quota of parent subvolume is a solution, but I won't address nor
>>>> discuss this here.
>>>>
>>>>
>>>> - Proposal
>>>>    (1) deletion of subvolume
>>>>
>>>>     I want to change the default behavior to allow a user to delete their own
>>>>     subvolumes.
>>>>     
>>>>     This is not the same behavior as when user_subvol_rm_alowed mount option is
>>>>     specified; in that case a user can delete the subvolume to which they have
>>>>     write+exec right.
>>>>     
>>>>     Since snapshot creation is already restricted to the subvolume owner, it is
>>>>     consistent that only the owner of the subvolume (or root) can delete it.
>>>>     
>>>>     The implementation should be straightforward.
>>>
>>> Personally speaking, I prefer to do the complex owner check in user daemon.
>>>
>>> And do the privilege in user daemon (call it btrfsd for example).
>>>
>>> So btrfs-progs will works in 2 modes, if root calls it, do as it used to do.
>>> If normal user calls it, proxy the request to btrfsd, and btrfsd does
>>> the privilege checking and call ioctl (with root privilege).
>>>
>>> Then no impact to kernel, all complex work is done in user space.
>> Exactly how hard is it to just check ownership of the root inode of a
>> subvolume from the ioctl context?  You could just as easily push all the
>> checking out to the VFS layer by taking an open fd for the subvolume
>> root (and probably implicitly closing it) instead of taking a path, and
>> that would give you all the benefits of ACL's and whatever security
>> modules the local system is using.  Alternatively, quit treating
>> subvolumes specially for `unlink()`, and make them behave like regular
>> directories (and thus avoid the one possible complication resulting from
>> home directories being subvolumes but needing to be owned by the users).
> 
> Thanks to all for the comments.
> Let me explain my stance clearly:
> 
> I'm for limiting the subvolume operation to btrfs-progs commands since a
> subvolume looks like a regular directory, but is different in many points.
> This is the reason I want to fix not only "sub delete" but also "sub show/list" etc.
I agree that it needs to behave more sanely, but the approach of 
treating it differently from a directory is the entire cause of this 
issue in the first place.  If the original implementation had just 
treated them as regular directories once created (with the obvious 
caveat regarding read-only snapshots), we wouldn't be having this 
conversation right now, except possibly regarding fetching info about 
subvolumes.

Personally, I would really rather just let unlink() work the same on 
subvolumes as it does with directories, and only need special handling 
for creation and metadata retrieval (with the possible option of 'quick' 
deletion through a btrfs-progs command).
> 
> I think "sub delete" should be the counterpart of "sub create" and "sub snap".
> Since "sub snap" can be used for the subvolume the user owns, "sub delete"
> should be allowed to the suvolume the user owns too. Also, using these
> commands enables an instant creation/deletion of a  subvolume/snapshot, which
> is one of the features of btrfs.
Somewhat OT, but the only operation that's remotely 'instant' is 
creating an empty subvolume.  Snapshot creation has to walk the tree in 
the subvolume being snapshotted, which can take a long time (and as a 
result of it's implementation, also means BTRFS snapshots are _not_ 
atomic).  Subvolume deletion has to do a bunch of cleanup work in the 
background (though it may be fairly quick if it was a snapshot and the 
source subvolume hasn't changed much).
> 
> Regards,
> Tomohiro
>>
>> <rant>
>> This whole 'just make it a daemon' thing that's becoming the standard
>> solution everywhere these days is crap in most cases (note that I'm not
>> just singling out this proposal here).  Just because systemd does it
>> doesn't make it a good idea, especially when it makes things like this
>> far more opaque than they really need to be, which in turn makes it a
>> pain in the arse to debug _anything_ to do with permissions beyond basic
>> file access.
>> </rant>
>>
>> In almost all cases, the desired check is to allow root, possibly admin
>> users who have filesystem management permissions (usually exactly one
>> group), and the owner.  That is trivial to implement with a single ACL
>> entry and basic filesystem permissions without needing some special
>> daemon to check them provided we route things through the VFS layer
>> somehow.  In the rare cases that that's not what's wanted, then the
>> system is almost certainly using an LSM, and the checks can be made ther>>
>>>>
>>>>    (2) getting subvolume/quota info
>>>>
>>>>     TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
>>>>     (sub show/list, qgroup show etc.). This requires the root permission.
>>>>     
>>>>     The easiest way to allow a user to get subvolume/quota info is just relaxing
>>>>     the permission of TREE_SEARCH. However, since all the tree information (inc.
>>>>     file name) will be exposed, this poses a sequrity risk and is not acceptable.
>>>>     
>>>>     So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
>>>>     a normal user, which return only the info readable by the user:
>>>>     
>>>>      [subvolume info]
>>>>       Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
>>>>      right by searching FS/FILE tree and comparing it with caller's uid.
>>>>     
>>>>      [quota info]
>>>>       Same as above, but mainly search QUOTA tree and only returns level 0 info.
>>>>     
>>>>     Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
>>>>     needs to be relaxed for the user who has read access to the subvolume.
>>>
>>> Same method can apply to this either.
>>>
>>> Thanks,
>>> Qu
>>>>
>>>>
>>>> - Summary of Proposal
>>>>    - Change the default behavior to allow a user to delete their own subvolumes
>>>>    - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes

Re: [RFC] Improve subvolume usability for a normal user

[Duncan]

Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
excerpted:

> Somewhat OT, but the only operation that's remotely 'instant' is
> creating an empty subvolume.  Snapshot creation has to walk the tree in
> the subvolume being snapshotted, which can take a long time (and as a
> result of it's implementation, also means BTRFS snapshots are _not_
> atomic). Subvolume deletion has to do a bunch of cleanup work in the
> background (though it may be fairly quick if it was a snapshot and the
> source subvolume hasn't changed much).

Indeed, while btrfs in general has taken a strategy of making /creating/ 
snapshots and subvolumes fast, snapshot deletion in particular can take 
some time[1].

And in that regard a question just occurred to me regarding this whole 
very tough problem of a user being able to create but not delete 
subvolumes and snapshots:

Given that at least snapshot deletion (not so sure about non-snapshot 
subvolume deletion, tho I strongly suspect it would depend on the number 
of cross-subvolume reflinks) is already a task that can take some time, 
why /not/ just bite the bullet and make the behavior much more like the 
directory deletion, given that subvolumes already behave much like 
directories.  Yes, for non-root, that /does/ mean tracing the entire 
subtree and checking permissions, and yes, that's going to take time and 
lower performance somewhat, but subvolume and in particular snapshot 
deletion is already an operation that takes time, so this wouldn't be 
unduly changing the situation, and it would eliminate the entire class of 
security issues that come with either asymmetrically restricting deletion 
(but not creation) to root on the one hand, or possible data loss due to 
allowing a user to delete a subvolume they couldn't delete were it an 
ordinary directory due to not owning stuff further down the tree.

---
[1] Based on comments and reports here.  My own use-case doesn't involve 
snapshots/subvolumes.

-- 
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] Improve subvolume usability for a normal user

[Misono, Tomohiro]

On 2017/12/07 11:56, Duncan wrote:
> Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
> excerpted:
> 
>> Somewhat OT, but the only operation that's remotely 'instant' is
>> creating an empty subvolume.  Snapshot creation has to walk the tree in
>> the subvolume being snapshotted, which can take a long time (and as a
>> result of it's implementation, also means BTRFS snapshots are _not_
>> atomic). Subvolume deletion has to do a bunch of cleanup work in the
>> background (though it may be fairly quick if it was a snapshot and the
>> source subvolume hasn't changed much).
> 
> Indeed, while btrfs in general has taken a strategy of making /creating/ 
> snapshots and subvolumes fast, snapshot deletion in particular can take 
> some time[1].
> 
> And in that regard a question just occurred to me regarding this whole 
> very tough problem of a user being able to create but not delete 
> subvolumes and snapshots:
> 
> Given that at least snapshot deletion (not so sure about non-snapshot 
> subvolume deletion, tho I strongly suspect it would depend on the number 
> of cross-subvolume reflinks) is already a task that can take some time, 
> why /not/ just bite the bullet and make the behavior much more like the 
> directory deletion, given that subvolumes already behave much like 
> directories.  Yes, for non-root, that /does/ mean tracing the entire 
> subtree and checking permissions, and yes, that's going to take time and 
> lower performance somewhat, but subvolume and in particular snapshot 
> deletion is already an operation that takes time, so this wouldn't be 
> unduly changing the situation, and it would eliminate the entire class of 
> security issues that come with either asymmetrically restricting deletion 
> (but not creation) to root on the one hand,

> or possible data loss due to 
> allowing a user to delete a subvolume they couldn't delete were it an 
> ordinary directory due to not owning stuff further down the tree.

But, this is also the very reason I'm for "sub del" instead of unlink().
Since snapshot creation won't check the permissions of the containing files/dirs,
it can copy a directory which cannot be deleted by the user.
Therefore if we won't allow "sub del" for the user, he couldn't remove the snapshot.

> 
> ---
> [1] Based on comments and reports here.  My own use-case doesn't involve 
> snapshots/subvolumes.
> 

Re: [RFC] Improve subvolume usability for a normal user

[Duncan]

Misono, Tomohiro posted on Thu, 07 Dec 2017 16:15:47 +0900 as excerpted:

> On 2017/12/07 11:56, Duncan wrote:
>> Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
>> excerpted:
>> 
>>> Somewhat OT, but the only operation that's remotely 'instant' is
>>> creating an empty subvolume.  Snapshot creation has to walk the tree
>>> in the subvolume being snapshotted, which can take a long time (and as
>>> a result of it's implementation, also means BTRFS snapshots are _not_
>>> atomic). Subvolume deletion has to do a bunch of cleanup work in the
>>> background (though it may be fairly quick if it was a snapshot and the
>>> source subvolume hasn't changed much).
>> 
>> Indeed, while btrfs in general has taken a strategy of making
>> /creating/ snapshots and subvolumes fast, snapshot deletion in
>> particular can take some time[1].
>> 
>> And in that regard a question just occurred to me regarding this whole
>> very tough problem of a user being able to create but not delete
>> subvolumes and snapshots:
>> 
>> Given that at least snapshot deletion (not so sure about non-snapshot
>> subvolume deletion, tho I strongly suspect it would depend on the
>> number of cross-subvolume reflinks) is already a task that can take
>> some time, why /not/ just bite the bullet and make the behavior much
>> more like the directory deletion, given that subvolumes already behave
>> much like directories.  Yes, for non-root, that /does/ mean tracing the
>> entire subtree and checking permissions, and yes, that's going to take
>> time and lower performance somewhat, but subvolume and in particular
>> snapshot deletion is already an operation that takes time, so this
>> wouldn't be unduly changing the situation, and it would eliminate the
>> entire class of security issues that come with either asymmetrically
>> restricting deletion (but not creation) to root on the one hand,
> 
>> or possible data loss due to allowing a user to delete a subvolume they
>> couldn't delete were it an ordinary directory due to not owning stuff
>> further down the tree.
> 
> But, this is also the very reason I'm for "sub del" instead of unlink().
> Since snapshot creation won't check the permissions of the containing
> files/dirs, it can copy a directory which cannot be deleted by the user.
> Therefore if we won't allow "sub del" for the user, he couldn't remove
> the snapshot.

Maybe snapshot creation /should/ check all that, in ordered to allow 
permissions to allow deletion.

Tho that would unfortunately increase the creation time, and btrfs is 
currently optimized for fast creation time.

Hmm... What about creating a "temporary" snapshot if not root, then 
walking the tree to check perms and deleting it without ever showing it 
to userspace if the perms wouldn't let the user delete it.  That would 
retain fast creation logic, tho it wouldn't show up until the perms walk 
was completed.

-- 
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] Improve subvolume usability for a normal user

[Austin S. Hemmelgarn]

On 2017-12-07 06:55, Duncan wrote:
> Misono, Tomohiro posted on Thu, 07 Dec 2017 16:15:47 +0900 as excerpted:
> 
>> On 2017/12/07 11:56, Duncan wrote:
>>> Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
>>> excerpted:
>>>
>>>> Somewhat OT, but the only operation that's remotely 'instant' is
>>>> creating an empty subvolume.  Snapshot creation has to walk the tree
>>>> in the subvolume being snapshotted, which can take a long time (and as
>>>> a result of it's implementation, also means BTRFS snapshots are _not_
>>>> atomic). Subvolume deletion has to do a bunch of cleanup work in the
>>>> background (though it may be fairly quick if it was a snapshot and the
>>>> source subvolume hasn't changed much).
>>>
>>> Indeed, while btrfs in general has taken a strategy of making
>>> /creating/ snapshots and subvolumes fast, snapshot deletion in
>>> particular can take some time[1].
>>>
>>> And in that regard a question just occurred to me regarding this whole
>>> very tough problem of a user being able to create but not delete
>>> subvolumes and snapshots:
>>>
>>> Given that at least snapshot deletion (not so sure about non-snapshot
>>> subvolume deletion, tho I strongly suspect it would depend on the
>>> number of cross-subvolume reflinks) is already a task that can take
>>> some time, why /not/ just bite the bullet and make the behavior much
>>> more like the directory deletion, given that subvolumes already behave
>>> much like directories.  Yes, for non-root, that /does/ mean tracing the
>>> entire subtree and checking permissions, and yes, that's going to take
>>> time and lower performance somewhat, but subvolume and in particular
>>> snapshot deletion is already an operation that takes time, so this
>>> wouldn't be unduly changing the situation, and it would eliminate the
>>> entire class of security issues that come with either asymmetrically
>>> restricting deletion (but not creation) to root on the one hand,
>>
>>> or possible data loss due to allowing a user to delete a subvolume they
>>> couldn't delete were it an ordinary directory due to not owning stuff
>>> further down the tree.
>>
>> But, this is also the very reason I'm for "sub del" instead of unlink().
>> Since snapshot creation won't check the permissions of the containing
>> files/dirs, it can copy a directory which cannot be deleted by the user.
>> Therefore if we won't allow "sub del" for the user, he couldn't remove
>> the snapshot.
> 
> Maybe snapshot creation /should/ check all that, in ordered to allow
> permissions to allow deletion.
> 
> Tho that would unfortunately increase the creation time, and btrfs is
> currently optimized for fast creation time.
> 
> Hmm... What about creating a "temporary" snapshot if not root, then
> walking the tree to check perms and deleting it without ever showing it
> to userspace if the perms wouldn't let the user delete it.  That would
> retain fast creation logic, tho it wouldn't show up until the perms walk
> was completed.
> 
I would argue that it makes more sense to keep snapshot creation as is, 
keep the subvolume deletion command as is (with some proper permissions 
checks of course), and just make unlink() work for subvolumes like it 
does for directories.

Re: [RFC] Improve subvolume usability for a normal user

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 3673 bytes --]

On Thu, Dec 07, 2017 at 07:21:46AM -0500, Austin S. Hemmelgarn wrote:
> On 2017-12-07 06:55, Duncan wrote:
> >Misono, Tomohiro posted on Thu, 07 Dec 2017 16:15:47 +0900 as excerpted:
> >
> >>On 2017/12/07 11:56, Duncan wrote:
> >>>Austin S. Hemmelgarn posted on Wed, 06 Dec 2017 07:39:56 -0500 as
> >>>excerpted:
> >>>
> >>>>Somewhat OT, but the only operation that's remotely 'instant' is
> >>>>creating an empty subvolume.  Snapshot creation has to walk the tree
> >>>>in the subvolume being snapshotted, which can take a long time (and as
> >>>>a result of it's implementation, also means BTRFS snapshots are _not_
> >>>>atomic). Subvolume deletion has to do a bunch of cleanup work in the
> >>>>background (though it may be fairly quick if it was a snapshot and the
> >>>>source subvolume hasn't changed much).
> >>>
> >>>Indeed, while btrfs in general has taken a strategy of making
> >>>/creating/ snapshots and subvolumes fast, snapshot deletion in
> >>>particular can take some time[1].
> >>>
> >>>And in that regard a question just occurred to me regarding this whole
> >>>very tough problem of a user being able to create but not delete
> >>>subvolumes and snapshots:
> >>>
> >>>Given that at least snapshot deletion (not so sure about non-snapshot
> >>>subvolume deletion, tho I strongly suspect it would depend on the
> >>>number of cross-subvolume reflinks) is already a task that can take
> >>>some time, why /not/ just bite the bullet and make the behavior much
> >>>more like the directory deletion, given that subvolumes already behave
> >>>much like directories.  Yes, for non-root, that /does/ mean tracing the
> >>>entire subtree and checking permissions, and yes, that's going to take
> >>>time and lower performance somewhat, but subvolume and in particular
> >>>snapshot deletion is already an operation that takes time, so this
> >>>wouldn't be unduly changing the situation, and it would eliminate the
> >>>entire class of security issues that come with either asymmetrically
> >>>restricting deletion (but not creation) to root on the one hand,
> >>
> >>>or possible data loss due to allowing a user to delete a subvolume they
> >>>couldn't delete were it an ordinary directory due to not owning stuff
> >>>further down the tree.
> >>
> >>But, this is also the very reason I'm for "sub del" instead of unlink().
> >>Since snapshot creation won't check the permissions of the containing
> >>files/dirs, it can copy a directory which cannot be deleted by the user.
> >>Therefore if we won't allow "sub del" for the user, he couldn't remove
> >>the snapshot.
> >
> >Maybe snapshot creation /should/ check all that, in ordered to allow
> >permissions to allow deletion.
> >
> >Tho that would unfortunately increase the creation time, and btrfs is
> >currently optimized for fast creation time.
> >
> >Hmm... What about creating a "temporary" snapshot if not root, then
> >walking the tree to check perms and deleting it without ever showing it
> >to userspace if the perms wouldn't let the user delete it.  That would
> >retain fast creation logic, tho it wouldn't show up until the perms walk
> >was completed.
> >
> I would argue that it makes more sense to keep snapshot creation as
> is, keep the subvolume deletion command as is (with some proper
> permissions checks of course), and just make unlink() work for
> subvolumes like it does for directories.

   Definitely this.

   Principle of least surprise.

   Hugo.

-- 
Hugo Mills             | ... one ping(1) to rule them all, and in the
hugo@... carfax.org.uk | darkness bind(2) them.
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                                Illiad

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] Improve subvolume usability for a normal user

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1496 bytes --]



On 2017年12月06日 20:39, Austin S. Hemmelgarn wrote:
> Somewhat OT, but the only operation that's remotely 'instant' is
> creating an empty subvolume.  Snapshot creation has to walk the tree in
> the subvolume being snapshotted, which can take a long time (and as a
> result of it's implementation, also means BTRFS snapshots are _not_
> atomic).

Nope, this is not true.

Btrfs from the very beginning is designed to speed up snapshot.
The most obvious evidence is its extent tree design.

When doing snapshot, btrfs only needs to increase reference of 2nd
highest level tree blocks of original snapshot, other than "walking the
tree".
(If tree root level is 2, then level 2 node is copied, while all
reference of level 1 tree blocks get increased)

This design hugely speeds up snapshot creation, making snapshot
obviously faster than any block level snapshot.

Although this design obviously slows down backref walk.
As btrfs must do a full walk until tree root, to see if any tree blocks
between leaf and root is shared with another snapshot.


And for btrfs snapshot atomic thing, it's just because btrfs delayed
snapshot creation to transaction commit time, and buffered write is not
triggered for snapshot creation, so it makes snapshot not so atomic.

Thanks,
Qu

>  Subvolume deletion has to do a bunch of cleanup work in the
> background (though it may be fairly quick if it was a snapshot and the
> source subvolume hasn't changed much).


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [RFC] Improve subvolume usability for a normal user

[Marat Khalili]

On 07/12/17 08:27, Qu Wenruo wrote:
> When doing snapshot, btrfs only needs to increase reference of 2nd
> highest level tree blocks of original snapshot, other than "walking the
> tree".
> (If tree root level is 2, then level 2 node is copied, while all
> reference of level 1 tree blocks get increased)

Out of curiosity, how does it interacts with nocow files? Does every 
write to these files involves backref walk?

--

With Best Regards,
Marat Khalili

Re: [RFC] Improve subvolume usability for a normal user

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1184 bytes --]



On 2017年12月07日 15:32, Marat Khalili wrote:
> On 07/12/17 08:27, Qu Wenruo wrote:
>> When doing snapshot, btrfs only needs to increase reference of 2nd
>> highest level tree blocks of original snapshot, other than "walking the
>> tree".
>> (If tree root level is 2, then level 2 node is copied, while all
>> reference of level 1 tree blocks get increased)
> 
> Out of curiosity, how does it interacts with nocow files? Does every
> write to these files involves backref walk?

For details I need to check the code.
But at least, partial backref walk must be done.

For "partial" I mean it doesn't need to get every root referring to the
file extent, it only needs to check if the extent is only referred by
current inode and offset.

If any other inode (or even the same inode with different offset) is
found, then the walk is finished.
(The same walk we do to determine the FIEMAP_EXTENT_SHARED flag)

In contract, for qgroup and balance, the full backref walk is done,
where btrfs needs every root involved (for qgroup) or even every inode
and offset involved (for balance).

Thanks,
Qu

> 
> -- 
> 
> With Best Regards,
> Marat Khalili


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [RFC] Improve subvolume usability for a normal user

[Goffredo Baroncelli]

On 12/05/2017 09:25 AM, Misono, Tomohiro wrote:
> Hello all,
> 
> I want to address some issues of subvolume usability for a normal user.
> i.e. a user can create subvolumes, but
>  - Cannot delete their own subvolume (by default)
>  - Cannot tell subvolumes from directories (in a straightforward way)
>  - Cannot check the quota limit when qgroup is enabled
> 
> Here I show the initial thoughts and approaches to this problem.
> I want to check if this is a right approach or not before I start writing code.
> 
> Comments are welcome.
> Tomohiro Misono
> 
> ==========
> - Goal and current problem
> The goal of this RFC is to give a normal user more control to their own subvolumes.
> Currently the control to subvolumes for a normal user is restricted as below: 
> 
> +-------------+------+------+
> |   command   | root | user |
> +-------------+------+------+
> | sub create  | Y    | Y    |
> | sub snap    | Y    | Y    |
> | sub del     | Y    | N    |
> | sub list    | Y    | N    |
> | sub show    | Y    | N    |
> | qgroup show | Y    | N    |
> +-------------+------+------+
> 

I suggest to consider also 

   btrfs fi usage

see my patches [1] and [2]

> In short, I want to change this as below in order to improve user's usability:
> 
> +-------------+------+--------+
> |   command   | root | user   |
> +-------------+------+--------+
> | sub create  | Y    | Y      |
> | sub snap    | Y    | Y      |
> | sub del     | Y    | N -> Y |
> | sub list    | Y    | N -> Y |
> | sub show    | Y    | N -> Y |
> | qgroup show | Y    | N -> Y |
> +-------------+------+--------+
> 
> In words,
> (1) allow deletion of subvolume if a user owns it, and 
I commented this in another part of this thread. My suggestion is to extend unlink(2) to remove a subvolume if it is empty, and if the user has needed privileges. This would be fully POSIX compliant.

[....]

> 
>  (2) getting subvolume/quota info
> 
>   TREE_SEARCH ioctl is used to retrieve the subvolume/quota info by btrfs-progs
>   (sub show/list, qgroup show etc.). This requires the root permission.
>   
>   The easiest way to allow a user to get subvolume/quota info is just relaxing
>   the permission of TREE_SEARCH. However, since all the tree information (inc.
>   file name) will be exposed, this poses a sequrity risk and is not acceptable.
>   
>   So, I want to introduce 2 ioctls to get subvolume/quota info respectively for
>   a normal user, which return only the info readable by the user:
>   
>    [subvolume info]
>     Mainly search ROOT tree for ROOT_ITEM/ROOT_BACKREF, but check its read
>    right by searching FS/FILE tree and comparing it with caller's uid.

Is it POSIX compliant ? Currently I can't access an entry if its parent directory doesn't have the 'x' bit enabled.
>From another point of view, a subvolume is more like a "mount --bind". And I can know which and where devices are mounted looking at /proc/self/mountinfo.

I fear that we should build the path at the kernel level, then check (again at the kernel level) if all path elements are accessible by the user, and then returning the full path to the user. I am incline to think that accessing element by element (on the basis of the parent directory access bit), would be a nightmare from posix compliance POV.

A more simple approach would be traversing all the filesystem tree (via opendir/readdir) and return all the idem with inode == 256 (something like 'find / -inum 256'). Even tough it would be very inefficient


>   
>    [quota info]
>     Same as above, but mainly search QUOTA tree and only returns level 0 info.
>   
>   Also, in order to construct subvolume path, permission of INO_LOOKUP ioctl
>   needs to be relaxed for the user who has read access to the subvolume.
> 
> 
> - Summary of Proposal
>  - Change the default behavior to allow a user to delete their own subvolumes
>  - Add 2 new non-root ioctls to get subvolume/quota info for accessible subvolumes
> ==========
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[1] https://www.spinics.net/lists/linux-btrfs/msg69564.html
[2] https://www.spinics.net/lists/linux-btrfs/msg69566.html

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