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.
Seriously. Even correct first->pprev is not needed. Look:
struct hlist_head head = unmounted;
if (likely(hlist_empty(&head))) // no dereferences of ppre *or* next
sod off
head.first->pprev = &head.first; // pprev of the first is valid now
INIT_HLIST_HEAD(&unmounted);
hlist_for_each_entry(mnt, &head, mnt_hash)
if (mnt->mnt_ex_mountpoint.mnt)
mntget(mnt->mnt_ex_mountpoint.mnt);
// only needed ->next for that loop
...
while (!hlist_empty(&head)) {
mnt = hlist_entry(head.first, struct mount, mnt_hash);
hlist_del_init(&mnt->mnt_hash);
...
}
Now, we certainly need pprev of the first to be correct for hlist_del_init()
to work. What we do not need is correctness of pprev of the second as we
call hlist_del_init(). And _after_ hlist_del_init() pprev of what used to
be the second (the first, now) is healed.
So we actually are OK here.
--
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
