Re: [PATCH] mremap: enforce rmap src/dst vma ordering in case ofvma_merge succeeding in copy_vma

From: Andrea Arcangeli
Date: Fri Nov 04 2011 - 23:08:26 EST


On Fri, Nov 04, 2011 at 07:21:28PM -0700, Hugh Dickins wrote:
> I found Andrea's "anon_vma_merge" reply very hard to understand; but
> it looks like he now accepts that it was mistaken, or on the wrong
> track at least...

No matter how we get the order right, we still need to reverse the
order in case of error without taking the lock. So even allocating a
new vma every time wouldn't be enough to get out of the ordering
games (it would be enough in the non-error path of course...).

So there are a couple of ways:

1) Keep my patch (adjust comment) and add a second ordering call in
the error path. Cleanup the *vmap case.

2) Always allocate a new vma, merge later, and still keep my patch for
reversing the order in the error path only (not an huge improvement
if we still have to reverse the order). So this now looks the worst
option at the light of the error path which would give
trouble by going the opposite way... again.

3) Return to your fix that takes the anon_vma lock during the pte
moves

Fixing my patch requires just a one liner to fix the error path, it's
not like the patch was wrong in fact it reduced the window even more,
it just missed one liner in the error path.

But it's still doing reordering. Which I think is safe and not
fundamentally different in ordering terms by the old anon_vma logic
before _chain (which is why this bug could have triggered before
too). But certainly more complex than taking the anon_vma lock around
every pagetable move, that's for sure. fork will still relay on the
ordering but fork has a super easy life compared to mremap which goes
both ways and has vma_merge in it too which makes the vma order non
deterministic.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/