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 > 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. Or we can move cleanup of I_LINKABLE to drop_nlink()/clean_nlink() > unlock the parent > find the parent of destination > lock it > look the destination up > call vfs_link > decrement child's i_nlink - vfs_link has bumped it > unlock the parent of destination > > I do realize it can lead to leaked link count on a crash, obviously... No such problem with I_LINKABLE. Thanks, Amir.
