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@xxxxxxxxxxxxxxx > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
