Re: bug: data corruption introduced by commit 83d116c53058 ("mm: fix double page fault on arm64 if PTE_AF is cleared")

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Tue, Feb 11, 2020 at 12:55:50PM -0500, Jeff Moyer wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
> 
> > On Tue, Feb 11, 2020 at 11:44:06AM -0500, Jeff Moyer wrote:
> >> Hi, Justin,
> >> 
> >> Justin He <Justin.He@arm.com> writes:
> >> >> Thanks for the report. But this commit 83d116c53058 doesn't add the
> >> >> new clear_page code path. Besides the pte_mkyoung part, It just refines
> >> >> the codes(no functional change) and add a WARN_ON_ONCE to indicate
> >> >> there is any obscure case before.
> >> >
> >> > I can't reproduce it with your provided test file on my arm64 qemu with
> >> > a pmem device.
> >> > Could you do me a favor that just revert 83d116c53058 but keep that
> >> > WARN_ON_ONCE after clear_page()? Is there any difference?
> >> > Thanks for your help
> >> 
> >> Below is the patch I used to put the WARN_ON_ONCE after the clear_page,
> >> just to be sure that's what you intended.  So with 83d116c53058
> >> reverted, and the below patch applied, the WARN_ON_ONCE does not
> >> trigger.
> >
> > I cannot explain this. There is no locking to prevent the same scenario
> > before. It might be an timing difference.
> >
> > Could try to put a delay before the copy to make race window larger?
> 
> I reverted my change to the reproducer, and now it triggers the warning.

I'm not sure I follow.

My understanding is that you failed to reproduce the issue with
83d116c53058 reverted and WARN_ON_ONCE() placed.

My ask was to try to put some mdelay() just before
__copy_from_user_inatomic(). The mdelay() may help with reproducing the
issue on the old code.

If the bug still fails to reproduce I may misunderstand the source of the
bug and need to look further.

-- 
 Kirill A. Shutemov