linux-kernel - Re: [PATCH v2] mm/memory.c: do_fault: avoid usage of stale vm_area

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 3 Mar 2019 02:27:02 -0500 (EST)
From:   Jan Stancek <jstancek@...hat.com>
To:     Andrea Arcangeli <aarcange@...hat.com>, peterz@...radead.org
Cc:     linux-mm@...ck.org, akpm@...ux-foundation.org, willy@...radead.org,
        riel@...riel.com, mhocko@...e.com,
        ying huang <ying.huang@...el.com>,
        jrdr linux <jrdr.linux@...il.com>, jglisse@...hat.com,
        aneesh kumar <aneesh.kumar@...ux.ibm.com>, david@...hat.com,
        raquini@...hat.com, rientjes@...gle.com, kirill@...temov.name,
        mgorman@...hsingularity.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm/memory.c: do_fault: avoid usage of stale
 vm_area_struct



----- Original Message -----
> Hello Jan,
> 
> On Sat, Mar 02, 2019 at 07:19:39PM +0100, Jan Stancek wrote:
> > +	struct mm_struct *vm_mm = READ_ONCE(vma->vm_mm);
> 
> The vma->vm_mm cannot change under gcc there, so no need of
> READ_ONCE. The release of mmap_sem has release semantics so the
> vma->vm_mm access cannot be reordered after up_read(mmap_sem) either.
> 
> Other than the above detail:
> 
> Reviewed-by: Andrea Arcangeli <aarcange@...hat.com>

Thank you for review, I dropped READ_ONCE and sent v3 with your
Reviewed-by included. I also successfully re-ran tests over-night.

> Would this not need a corresponding WRITE_ONCE() in vma_init() ?

There's at least 2 context switches between, so I think it wouldn't matter.
My concern was gcc optimizing out vm_mm, and vma->vm_mm access happening only
after do_read_fault().