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(). -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
