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

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

On Thu, Dec 18, 2014 at 08:01:10PM +0000, Al Viro wrote:
> On Thu, Dec 18, 2014 at 10:57:19AM -0600, Eric W. Biederman wrote:
> > 
> > While reviewing the code of umount_tree I realized that when we append
> > to a preexisting unmounted list we do not change pprev of the former
> > first item in the list.
> > 
> > Which means later in namespace_unlock hlist_del_init(&mnt->mnt_hash) on
> > the former first item of the list will stomp unmounted.first leaving
> > it set to some random mount point which we are likely to free soon.
> > 
> > This isn't likely to hit, but if it does I don't know how anyone could
> > track it down.
> 
> All you need for that kind of loop to work is correct -next on all elements.

Mind you, I'm not saying that this isn't worth sanitizing.  Just that analysis
in your commit message is incorrect - we actually do _not_ stomp on memory
there.  It's brittle and it's only saved by having only very few things done
to that hlist, but the actual memory corruption does not happen.  Again, I'm
no defending that code; it's certaily better off making a properly-spliced
hlist rather than relying on precise sequence of operations in
namespace_unlock().