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
signature.asc
Description: Digital signature
