Re: [PATCH] mnt: Fix a memory stomp in umount

[Al Viro <viro@ZenIV.linux.org.uk>]

On Thu, Dec 18, 2014 at 01:24:26PM -0600, Eric W. Biederman wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > On Thu, Dec 18, 2014 at 9:07 AM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> >> Why is this piece of code using its own made up and buggy list handling in
> >> the first place? We have list functions for these things, exactly so that
> >> people shouldn't write buggy stuff by hand.
> >
> > Oh. Ok, I see what's going on. We have "list_splice()", but we don't
> > have the equivalent "hlist_splice()". So it's doing that by hand, and
> > did it badly.
> >
> > Al, this is your bug. I guess I can take the "manual hlist_splice" fix
> > from Eric, but I'm not really happy with it. There's a few other
> > places in that same commit where the list splice operation has been
> > open-coded.
> >
> > Mind taking a look?
> 
> It looks like we can pretty easily use mnt_list instead of mnt_hash,
> see below (note: the code is only compile tested).
> 
> While converting this to ordinary list helpers I found something
> strange.
> 
> In __propagate_umount we currently add the child to be unmounted in a
> different location in the list then we did before the conversion of
> mnt_hash to a hlist for rcu's accesses benefit.
> 
> Now maybe propagate_next handles this (I still need to read and
> understand that code) if not it looks like I may have found another bug,
> as it looks like today we can add a node to our list without propogating
> the unmount from the node.

Er...  Why would we want to reprocess it?  The loop goes through all nodes
getting events from ours; everything that gets events from _them_ included
at the same time they are.