From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2B65C2BA83 for ; Wed, 12 Feb 2020 14:22:12 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A41A0206D7 for ; Wed, 12 Feb 2020 14:22:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="EWXXl/43" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A41A0206D7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 36B346B0456; Wed, 12 Feb 2020 09:22:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 31CD76B0457; Wed, 12 Feb 2020 09:22:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20A3D6B0458; Wed, 12 Feb 2020 09:22:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0161.hostedemail.com [216.40.44.161]) by kanga.kvack.org (Postfix) with ESMTP id 04D2F6B0456 for ; Wed, 12 Feb 2020 09:22:11 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 8C4282490 for ; Wed, 12 Feb 2020 14:22:11 +0000 (UTC) X-FDA: 76481689662.17.hat02_302c4c6a5c90c X-HE-Tag: hat02_302c4c6a5c90c X-Filterd-Recvd-Size: 5894 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Wed, 12 Feb 2020 14:22:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581517330; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=l6I0g41yDLhSTrjQHy1OA2qeb6Z3WKVlh6dipgbutxg=; b=EWXXl/43923KeJh9lc/DxKnDjbbGQ27FEl2NDEexbAu+y+5TXhEemTcEQfKxvCHfrFtYen p7ARldCCeT7CMuWgi16NUZs4Dr+0L06brMt+B4ewW731DBpmAO6fgZzYeHjpfZoG48H9qz bFedtGHum+20nLKQojfkxa0XhVxpOx4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-123-iBjepxvsM5iNLyA6aKb4ZA-1; Wed, 12 Feb 2020 09:22:06 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DC5581005F72; Wed, 12 Feb 2020 14:22:04 +0000 (UTC) Received: from segfault.boston.devel.redhat.com (segfault.boston.devel.redhat.com [10.19.60.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 353125C1B0; Wed, 12 Feb 2020 14:22:04 +0000 (UTC) From: Jeff Moyer To: "Kirill A. Shutemov" Cc: Jia He , Catalin Marinas , "Kirill A. Shutemov" , linux-mm@kvack.org Subject: Re: bug: data corruption introduced by commit 83d116c53058 ("mm: fix double page fault on arm64 if PTE_AF is cleared") References: <20200211145158.5wt7nepe3flx25bj@box> <20200211224038.4u6au5jwki7lofpq@box> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 Date: Wed, 12 Feb 2020 09:22:03 -0500 In-Reply-To: <20200211224038.4u6au5jwki7lofpq@box> (Kirill A. Shutemov's message of "Wed, 12 Feb 2020 01:40:38 +0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: iBjepxvsM5iNLyA6aKb4ZA-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: "Kirill A. Shutemov" writes: > On Tue, Feb 11, 2020 at 11:27:36AM -0500, Jeff Moyer wrote: >> > The real solution would be to retry __copy_from_user_inatomic() under = ptl >> > if the first attempt fails. I expect it to be ugly. >>=20 >> So long as it's correct. :) > > The first attempt on the real solution is below. > > Yeah, this is ugly. Any suggestion on clearing up this mess is welcome. > > Jeff, could you give it a try? Yes, that patch appears to fix the problem. I wonder if we could remove the clear_page completely, though. I'd rather see the program segfault than operate on bad data. What do you think? -Jeff > > diff --git a/mm/memory.c b/mm/memory.c > index 0bccc622e482..e8bfdf0d9d1d 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -2257,7 +2257,7 @@ static inline bool cow_user_page(struct page *dst, = struct page *src, > =09bool ret; > =09void *kaddr; > =09void __user *uaddr; > -=09bool force_mkyoung; > +=09bool locked =3D false; > =09struct vm_area_struct *vma =3D vmf->vma; > =09struct mm_struct *mm =3D vma->vm_mm; > =09unsigned long addr =3D vmf->address; > @@ -2282,11 +2282,11 @@ static inline bool cow_user_page(struct page *dst= , struct page *src, > =09 * On architectures with software "accessed" bits, we would > =09 * take a double page fault, so mark it accessed here. > =09 */ > -=09force_mkyoung =3D arch_faults_on_old_pte() && !pte_young(vmf->orig_pt= e); > -=09if (force_mkyoung) { > +=09if (arch_faults_on_old_pte() && !pte_young(vmf->orig_pte)) { > =09=09pte_t entry; > =20 > =09=09vmf->pte =3D pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl); > +=09=09locked =3D true; > =09=09if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) { > =09=09=09/* > =09=09=09 * Other thread has already handled the fault > @@ -2310,18 +2310,37 @@ static inline bool cow_user_page(struct page *dst= , struct page *src, > =09 * zeroes. > =09 */ > =09if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) { > +=09=09if (locked) > +=09=09=09goto warn; > + > +=09=09/* Re-validate under PTL if the page is still mapped */ > +=09=09vmf->pte =3D pte_offset_map_lock(mm, vmf->pmd, addr, &vmf->ptl); > +=09=09locked =3D true; > +=09=09if (!likely(pte_same(*vmf->pte, vmf->orig_pte))) { > +=09=09=09/* The PTE changed under us. Retry page fault. */ > +=09=09=09ret =3D false; > +=09=09=09goto pte_unlock; > +=09=09} > + > =09=09/* > -=09=09 * Give a warn in case there can be some obscure > -=09=09 * use-case > +=09=09 * The same page can be mapped back since last copy attampt. > +=09=09 * Try to copy again under PTL. > =09=09 */ > -=09=09WARN_ON_ONCE(1); > -=09=09clear_page(kaddr); > +=09=09if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) { > +=09=09=09/* > +=09=09=09 * Give a warn in case there can be some obscure > +=09=09=09 * use-case > +=09=09=09 */ > +warn: > +=09=09=09WARN_ON_ONCE(1); > +=09=09=09clear_page(kaddr); > +=09=09} > =09} > =20 > =09ret =3D true; > =20 > pte_unlock: > -=09if (force_mkyoung) > +=09if (locked) > =09=09pte_unmap_unlock(vmf->pte, vmf->ptl); > =09kunmap_atomic(kaddr); > =09flush_dcache_page(dst);