Re: [PATCH] mm: fix potential anon_vma locking issue in mprotect()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 5 Sep 2012, Andrea Arcangeli wrote:
> On Tue, Sep 04, 2012 at 05:02:49PM -0700, Michel Lespinasse wrote:
> > On Tue, Sep 4, 2012 at 4:46 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> > > On Tue,  4 Sep 2012 16:39:49 -0700
> > > Michel Lespinasse <walken@xxxxxxxxxx> wrote:
> > >
> > >> This change fixes an anon_vma locking issue in the following situation:
> > >> - vma has no anon_vma
> > >> - next has an anon_vma
> > >> - vma is being shrunk / next is being expanded, due to an mprotect call
> > >>
> > >> We need to take next's anon_vma lock to avoid races with rmap users
> > >> (such as page migration) while next is being expanded.

Mmm, good catch by Michel.

I got interested in the history of this, and checked back: we had this
locking right before 2.6.34, when it got lost somewhere in the succession
of changes for the anon_vma chains.

In 2.6.33, we were also locking next anon_vma for the remove_next case,
and I got worried that Michel's patch might be incomplete: but no,
nowadays the unlinking of the anon_vma is handled further down, getting
that lock quite separately in unlink_anon_vmas() (from "anon_vma_merge").

Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>

> > >
> > > hm, OK.  How serious was that bug?  I'm suspecting "only needed in
> > > 3.7".
> 
> Agreed.
> 
> > That was my starting position as well. I'd expect the biggest issue
> > would be page migration races, and we do have assertions for that
> > case, and we've not been hitting them (that I know of). So, this
> > should not be a high frequency issue AFAICT.
> 
> I exclude it's reproducible with real load too, the window is far too
> small.
> 
> A malicious load might reproduce it, but the worst case would be to
> trigger the BUG_ON assertion in migration_entry_to_page like you
> mentioned above or to "gracefully" hang in migration_entry_wait, or to
> trigger one of the BUG_ONs in split_huge_page with no risk of memory
> corruption or anything.

Yes, that malicious Mr Dave Jones is the most likely to trigger it.

I'll defer to you and Andrew and Michel as to whether the fix should
go to stable: you think not, and I agree it's a very narrow window,
whose only danger is triggering one of those BUG_ONs.

But certainly I'm glad that Michel and Andrew have separated it out
as a fix which can be applied independent of his series.

Hugh

> 
> The only two places in the VM that depends on full accuracy in finding
> all ptes from the rmap walk are remove_migration_ptes and
> split_huge_page and they both are (and must remain) robust enough not
> to generate memory corruption or any other adverse side effects if the
> rmap walk actually wasn't 100% accurate because of some race condition
> like in this case.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]