Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes: > 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(). Agreed. That code wanted to be a memory stomp but we got lucky and it couldn't figure out how to be. Which I guess in the grand scheme of things I am glad of. When I sent the patch I knew there was an issue and I figured I had better send a patch before my mind moved on and I forgot about the incorrectly spliced list. Eric -- 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
