Re: [PATCH v11 4/4] mm: fix double page fault on arm64 if PTE_AF is cleared

From: Catalin Marinas <catalin.marinas@arm.com>
To: Jia He <justin.he@arm.com>
Cc: Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	James Morse <james.morse@arm.com>, Marc Zyngier <maz@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Suzuki Poulose <Suzuki.Poulose@arm.com>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	hejianet@gmail.com, Kaly Xin <Kaly.Xin@arm.com>,
	nd@arm.com
Subject: Re: [PATCH v11 4/4] mm: fix double page fault on arm64 if PTE_AF is cleared
Date: Thu, 10 Oct 2019 17:45:21 +0100	[thread overview]
Message-ID: <20191010164520.GC40923@arrakis.emea.arm.com> (raw)
In-Reply-To: <20191009084246.123354-5-justin.he@arm.com>

On Wed, Oct 09, 2019 at 04:42:46PM +0800, Jia He wrote:
> When we tested pmdk unit test [1] vmmalloc_fork TEST3 on arm64 guest, there
> will be a double page fault in __copy_from_user_inatomic of cow_user_page.
> 
> To reproduce the bug, the cmd is as follows after you deployed everything:
> make -C src/test/vmmalloc_fork/ TEST_TIME=60m check
> 
> Below call trace is from arm64 do_page_fault for debugging purpose:
> [  110.016195] Call trace:
> [  110.016826]  do_page_fault+0x5a4/0x690
> [  110.017812]  do_mem_abort+0x50/0xb0
> [  110.018726]  el1_da+0x20/0xc4
> [  110.019492]  __arch_copy_from_user+0x180/0x280
> [  110.020646]  do_wp_page+0xb0/0x860
> [  110.021517]  __handle_mm_fault+0x994/0x1338
> [  110.022606]  handle_mm_fault+0xe8/0x180
> [  110.023584]  do_page_fault+0x240/0x690
> [  110.024535]  do_mem_abort+0x50/0xb0
> [  110.025423]  el0_da+0x20/0x24
> 
> The pte info before __copy_from_user_inatomic is (PTE_AF is cleared):
> [ffff9b007000] pgd=000000023d4f8003, pud=000000023da9b003,
>                pmd=000000023d4b3003, pte=360000298607bd3
> 
> As told by Catalin: "On arm64 without hardware Access Flag, copying from
> user will fail because the pte is old and cannot be marked young. So we
> always end up with zeroed page after fork() + CoW for pfn mappings. we
> don't always have a hardware-managed access flag on arm64."
> 
> This patch fixes it by calling pte_mkyoung. Also, the parameter is
> changed because vmf should be passed to cow_user_page()
> 
> Add a WARN_ON_ONCE when __copy_from_user_inatomic() returns error
> in case there can be some obscure use-case (by Kirill).
> 
> [1] https://github.com/pmem/pmdk/tree/master/src/test/vmmalloc_fork
> 
> Signed-off-by: Jia He <justin.he@arm.com>
> Reported-by: Yibo Cai <Yibo.Cai@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

My reviewed-by still stands. Thanks.

-- 
Catalin