On Tue, 13 Sept 2022 at 10:02, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Tue, Sep 13, 2022 at 7:38 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > > On Tue, Sep 13, 2022 at 03:04:17AM +0100, Al Viro wrote: > > > On Mon, Feb 21, 2022 at 09:20:02AM +0100, Miklos Szeredi wrote: > > > > > > [digging through the old piles of mail] > > > > > > Eyes-watering control flow in do_linkat() aside (it's bound to rot; too > > > much of it won't get any regression testing and it's convoluted enough > > > to break easily), the main problem I have with that is the DoS potential. > > > > > > You have a system-wide lock, and if it's stuck you'll get every damn > > > rename(2) stuck as well. Sure, having it taken only upon the race > > > with rename() (or unlink(), for that matter) make it harder to get > > > stuck with lock held, but that'll make the problem harder to reproduce > > > and debug... > > > > FWIW, how much trouble would we have if link(2) would do the following? > > > > find the parent of source > > Well, only if source is not AT_EMPTY_PATH The problem is not AT_EMPTY_PATH, but a disconnected dentry gotten through magic symlink. Theoretically link(2) should work for those, but I didn't try. > > > lock it > > look the child up > > verify it's a non-directory > > bump child's i_nlink > > all failure exits past that point decrement child's i_nlink > > No need to bump i_nlink. > Sufficient to set I_LINKABLE. > and clean it up on failure if i_nlink > 0. Bumping i_nlink seems hackish enough, currently all i_nlink modification is done by filesystem code. Setting I_LINKABLE seems safe for filesystems that have tmpfile, not otherwise. Thanks, Miklos
