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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2504C001CC for ; Wed, 17 Apr 2024 18:52:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5BD8D6B0087; Wed, 17 Apr 2024 14:52:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 546386B0088; Wed, 17 Apr 2024 14:52:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 370DB6B008A; Wed, 17 Apr 2024 14:52:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 16A6A6B0087 for ; Wed, 17 Apr 2024 14:52:28 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id BF9591A0F37 for ; Wed, 17 Apr 2024 18:52:27 +0000 (UTC) X-FDA: 82019919534.25.238093B Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf20.hostedemail.com (Postfix) with ESMTP id E06551C0004 for ; Wed, 17 Apr 2024 18:52:25 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=soleen-com.20230601.gappssmtp.com header.s=20230601 header.b=t5GgMg2V; spf=pass (imf20.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=pasha.tatashin@soleen.com; dmarc=pass (policy=none) header.from=soleen.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713379946; a=rsa-sha256; cv=none; b=2Cw3CPqpTX3DN/iEB/Bs8XI0VLCS6ffUGRA9BxKJTFTvLHTOl5X4ycosZxoiuuLCDnXD1X SqxU+ijyoUT2/a/B5H+GgeinjSWUp4zlyWki5gYjB4x56dUT+Pu82b5W+AbJytof9PhSmv FN6tzNfFD5JcFGTzuczrU/RNo7OU6rk= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=soleen-com.20230601.gappssmtp.com header.s=20230601 header.b=t5GgMg2V; spf=pass (imf20.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=pasha.tatashin@soleen.com; dmarc=pass (policy=none) header.from=soleen.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713379946; h=from:from:sender: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:dkim-signature; bh=eYY7Lz3nOoOLfIfGCItn7CVOAZGQHNOlGWi+/nIv2kg=; b=zZqdhnRBTyUd//Mr0XTDt239t2k3bLP6arcb7Y3AyHxg0LMgAkzd6W+ZfMQHzzgQAUUJU7 UVd38zUUVn2IxNy7mIBChpYhbc/5MJgvOyR8swWu8XndOP1gn6jG7UD6v0Swrt1ipV7IPO +ClDQgxyqTBHAK7L27UpMve93H4WzvY= Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-4349685c845so20091cf.0 for ; Wed, 17 Apr 2024 11:52:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=soleen-com.20230601.gappssmtp.com; s=20230601; t=1713379945; x=1713984745; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=eYY7Lz3nOoOLfIfGCItn7CVOAZGQHNOlGWi+/nIv2kg=; b=t5GgMg2V/vUag6Iv8TmIyhhe7qTzFByUx14Nmi53AYFSMTSKAASxOO4YwBWlbk7HvR Zn2unR+mAOLoK7p2wyypuzMMzRdoFwWGbq529XWF5zkwZtSzyNpAtchOhPp5qgdByb9W R3PGyGtbwIdP81Hn8QgVDRxjTGMsUwZmz9zDZ5cPnSntkkzIbtjGfcev5WrvQzJVbnKO 7o8baTS752bfjtJt1Y0o9xjkxWRGqW+ERXm/j7Dp6vhN4XhqSfYstYsb3EaOOdIqylDV kcDDMFtcnmoBBFzlJnxggG8wljeqS7b96RxOVF32sf4SrI5l0zCOmHljC6B5KUScP0Sd 160Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713379945; x=1713984745; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eYY7Lz3nOoOLfIfGCItn7CVOAZGQHNOlGWi+/nIv2kg=; b=DRXpygbfjPcxRgsubI/ZR7LQM8Yx5aQVlscub7jwPoDBXs5I0ssY3H1WF2ohiUg7yd Respm4oas49WbWc6yc7V6mjl9EZMHGrLhZw4wMdH1W640YADi+YyD2H6du2XX9o/MF9m 996XUPXoJVaOGEpDD5w5cWM3V0HdN4qOJmynFtqyKbWla7b6a1vYY9O2dl0iMPuPHzMD c0c6RhfN6H5kFW7IzCtEix6dUl1IYFSXvgL8A16S1RMvMPT/MZPy0yQ4XawtoQDuLBGo YhmaZW+bh6O/QSKY5vjjc5e4E+PceISGpgFhGgJte0UqNK1R6xUp38hCX0tU7rPDPD56 ZDOw== X-Forwarded-Encrypted: i=1; AJvYcCXg8UYSza8YCZR0Ngq3X2fdkfhrRrA7BMzcn5bKfFfuWhGVb35QPqOMtmn/mUEuPjOPtyI7ORtCN5mGnTlVtt3ZqY0= X-Gm-Message-State: AOJu0YyTXOPKjNLhP5LJO6bpYzbn5EREDenzL7aRpM4mp5Gfb3J2bUo1 zybuL20kllJgD5/oeEqQ2jEdlOpgWMFUjj5zPwDJOunN865M/44BJ1pH/tscrDelE3sJsK5v4C1 uRk3ElDjXcFK/39/Wj40Vpu5p5jEkaKOCeVYO4Q== X-Google-Smtp-Source: AGHT+IGEMcT+kNQWgvtNK+axIqQJUbJoPiQNPKJwzNdYAJVbhAlhbREAAzlJ+GHf0YdHgFHJ5RGTkjuImP1NIMEbenA= X-Received: by 2002:ac8:5909:0:b0:434:a5c9:b8e8 with SMTP id 9-20020ac85909000000b00434a5c9b8e8mr378901qty.54.1713379944999; Wed, 17 Apr 2024 11:52:24 -0700 (PDT) MIME-Version: 1.0 References: <20240415205259.2535077-1-peterx@redhat.com> In-Reply-To: From: Pasha Tatashin Date: Wed, 17 Apr 2024 14:51:48 -0400 Message-ID: Subject: Re: [PATCH] mm/page_table_check: Support userfault wr-protect entries To: Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, David Hildenbrand , Andrew Morton , Nadav Amit , Axel Rasmussen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: E06551C0004 X-Stat-Signature: hy8nc5b57bwtqmuppgxfo6tfkwg8uizn X-Rspam-User: X-HE-Tag: 1713379945-229717 X-HE-Meta: U2FsdGVkX1/f3gA8WqGHsZna0JiVYMPGXR78vHjLrjIQpCNnWsdfxiJL8rIXI0VNwRHUoOcA5+ze/Kj+P3QCxfFqaNuhQnWO9pfdPeyDXQ6QMDEW7SRgBHElknQj+IIxsRxLhPN+CDa8v1exhbVqBNxPjDkhm5r83NwXL7yHdkQYO2KzzTfWJhDGl2ZQKe2ybSSbLgGZ08Lcv9h3+bA+9FXgxoqkkcNBWKW/33iGWz++x2fHi9dvTPWHiYf/+1t76SYed4kUDBswqv82qFKqqiJH0kSC8mCUxuwo8udTK7naCuokbzvhO8SielLaZrZzFO8BQBVxKMI6wtF9oRJbXC9/LdwECTyL9RUHhEVnaBkG9wjH+tdGjF+cctFutNnDuIQPMSTaWhxCTsAtPy66rrOdYgJEp/K+/D3qNUE5gZi7AaaftLpi/6KApzSEPWgRUVNlCyBDqQWDWyokxqJvjEn6+FokrcHnRdrJWNdJ6fpMwqe85ZWbgFoP4ppqadLnXsNX5CxvNW6biKinvYtMTZWZD75/Sj6a2LaCKoIFivBoZCditJTb1PErbk0eeiIYp+aVLltvBnQtkTyiVXmZ1Y7X5f+ZIe5Z3uty8OMCb8zL96Zjip04VDyuTRkIXhlAOmowZGUpWjpLQnGsuT9H/6GhXsrXzwsRba3wfH4dzPWMdvwDCcvw3wj2c7Nk4/QheSHeOIIHyMCZN8wWBnoiYt0As0rEy4Brv9ih8mV8qaoLATj3rmiFqZB+IdwZuMXjmwz6TSjT43WbgvsQAgevCcxL1pahivE0MI3pKf0nADSH2LtwTju9ddbKRFy2vF9n0Fcb90vMh6dFxUlsFi9KgKWzldnUSHQKzPLRaT9/D2WBhaE5LfLzb4HlZ9b7eSSi7/yCZvQdC1xpludQvI0JOYEH1vzpgHBEvKLmJ+Ig3Ya1OBGPUFWDsD+WkZ9WrAFwTSPr1QEZxDjrDSBWomW 5CbR3n5R CE5et7iOMgMQWFaj0pdNI7HywpgDC6JQX+TAbeiB1le6cyFS4wjFav8BC85f46oXF5kDZRsUTiKPd5+jW/0BrpRUzp1ST9J9Emk/mnRYk1u4VzO9oJCBfmoSIadwwMINSs+fODifFSCgJxMmogVaHuU9QEiFbpmnyuTxDSoIgcqlPZUN21EPh/b9DiUt8fmw5sGIodqzRrGLOUw2UcCx56YuQFSHBAPxEq0q19LUxs8YPM1tLZ8IoDnJeCIW0Y8ELB6f5UDih57gYEDL7rRbNqiSU1BM7gRTXsars4K5rHfJCrirUjBfYlohz9/Cqv01o8m9fNT53YJXGeUhyB+RPgD1dKPuxOEiX+QG7W3KVJy73n589bc6CniC91PjrqY7X0uuj5P8MeZw1/rLyDxLTq1O2k9ux1b6zm/F3wMI+TvJSBqWPC99x8VbemowVbS0i5jNtruLEZTb9Q0+eDxIoaLXv4hRYtIuYMvD0PH3/OOck5JVppkezP3+Fqv+8KazT3PQwCwavRsM/yobYzB5VBtp83c/H+P5pNrR+9czpWCzEa+qW+MDI6GL13e3xjLO+LaYf 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: List-Subscribe: List-Unsubscribe: On Wed, Apr 17, 2024 at 1:17=E2=80=AFPM Pasha Tatashin wrote: > > On Wed, Apr 17, 2024 at 12:53=E2=80=AFPM Peter Xu wro= te: > > > > On Tue, Apr 16, 2024 at 05:34:50PM -0400, Pasha Tatashin wrote: > > > Hi Peter, > > > > Pasha, > > > > > > > > Thanks for this patch, I like this extra checking logic, my comments = below: > > > > Thanks for taking a look. > > > > > > > > On Mon, Apr 15, 2024 at 4:53=E2=80=AFPM Peter Xu = wrote: > > > > > > > > Allow page_table_check hooks to check over userfaultfd wr-protect c= riteria > > > > upon pgtable updates. The rule is no co-existance allowed for any = writable > > > > flag against userfault wr-protect flag. > > > > > > > > This should be better than c2da319c2e, where we used to only saniti= ze such > > > > issues during a pgtable walk, but when hitting such issue we don't = have a > > > > good chance to know where does that writable bit came from [1], so = that > > > > even the pgtable walk exposes a kernel bug (which is still helpful = on > > > > triaging) but not easy to track and debug. > > > > > > > > Now we switch to track the source. It's much easier too with the r= ecent > > > > introduction of page table check. > > > > > > > > There are some limitations with using the page table check here for > > > > userfaultfd wr-protect purpose: > > > > > > > > - It is only enabled with explicit enablement of page table check= configs > > > > and/or boot parameters, but should be good enough to track at lea= st > > > > syzbot issues, as syzbot should enable PAGE_TABLE_CHECK[_ENFORCED= ] for > > > > x86 [1]. We used to have DEBUG_VM but it's now off for most dist= ros, > > > > while distros also normally not enable PAGE_TABLE_CHECK[_ENFORCED= ], which > > > > is similar. > > > > > > > > - It conditionally works with the ptep_modify_prot API. It will = be > > > > bypassed when e.g. XEN PV is enabled, however still work for most= of the > > > > rest scenarios, which should be the common cases so should be goo= d > > > > enough. > > > > > > > > - Hugetlb check is a bit hairy, as the page table check cannot id= entify > > > > hugetlb pte or normal pte via trapping at set_pte_at(), because o= f the > > > > current design where hugetlb maps every layers to pte_t... For ex= ample, > > > > the default set_huge_pte_at() can invoke set_pte_at() directly an= d lose > > > > the hugetlb context, treating it the same as a normal pte_t. So f= ar it's > > > > fine because we have huge_pte_uffd_wp() always equals to pte_uffd= _wp() as > > > > long as supported (x86 only). It'll be a bigger problem when we'= ll > > > > define _PAGE_UFFD_WP differently at various pgtable levels, becau= se then > > > > one huge_pte_uffd_wp() per-arch will stop making sense first.. as= of now > > > > we can leave this for later too. > > > > > > > > This patch also removes commit c2da319c2e altogether, as we have so= mething > > > > better now. > > > > > > > > [1] https://lore.kernel.org/all/000000000000dce0530615c89210@google= .com/ > > > > > > > > Cc: Pasha Tatashin > > > > Signed-off-by: Peter Xu > > > > --- > > > > arch/x86/include/asm/pgtable.h | 18 +----------------- > > > > mm/page_table_check.c | 32 ++++++++++++++++++++++++++++++= +- > > > > > > Please add the new logic to: Documentation/mm/page_table_check.rst > > > > Will do. > > > > > > > > > 2 files changed, 32 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/= pgtable.h > > > > index 273f7557218c..65b8e5bb902c 100644 > > > > --- a/arch/x86/include/asm/pgtable.h > > > > +++ b/arch/x86/include/asm/pgtable.h > > > > @@ -388,23 +388,7 @@ static inline pte_t pte_wrprotect(pte_t pte) > > > > #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP > > > > static inline int pte_uffd_wp(pte_t pte) > > > > { > > > > - bool wp =3D pte_flags(pte) & _PAGE_UFFD_WP; > > > > - > > > > -#ifdef CONFIG_DEBUG_VM > > > > - /* > > > > - * Having write bit for wr-protect-marked present ptes is f= atal, > > > > - * because it means the uffd-wp bit will be ignored and wri= te will > > > > - * just go through. > > > > - * > > > > - * Use any chance of pgtable walking to verify this (e.g., = when > > > > - * page swapped out or being migrated for all purposes). It= means > > > > - * something is already wrong. Tell the admin even before = the > > > > - * process crashes. We also nail it with wrong pgtable setu= p. > > > > - */ > > > > - WARN_ON_ONCE(wp && pte_write(pte)); > > > > -#endif > > > > - > > > > - return wp; > > > > + return pte_flags(pte) & _PAGE_UFFD_WP; > > > > } > > > > > > > > static inline pte_t pte_mkuffd_wp(pte_t pte) > > > > diff --git a/mm/page_table_check.c b/mm/page_table_check.c > > > > index af69c3c8f7c2..d4eb1212f0f5 100644 > > > > --- a/mm/page_table_check.c > > > > +++ b/mm/page_table_check.c > > > > @@ -7,6 +7,8 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > +#include > > > > > > > > #undef pr_fmt > > > > #define pr_fmt(fmt) "page_table_check: " fmt > > > > @@ -182,6 +184,23 @@ void __page_table_check_pud_clear(struct mm_st= ruct *mm, pud_t pud) > > > > } > > > > EXPORT_SYMBOL(__page_table_check_pud_clear); > > > > > > > > +/* Whether the swap entry cached writable information */ > > > > +static inline bool swap_cached_writable(swp_entry_t entry) > > > > +{ > > > > + unsigned type =3D swp_type(entry); > > > > + > > > > + return type =3D=3D SWP_DEVICE_EXCLUSIVE_WRITE || > > > > + type =3D=3D SWP_MIGRATION_WRITE; This breaks linux-next build: mm/page_table_check.c: In function =E2=80=98swap_cached_writable=E2=80=99: mm/page_table_check.c:192:24: error: =E2=80=98SWP_DEVICE_EXCLUSIVE_WRITE=E2= =80=99 undeclared (first use in this function) 192 | return type =3D=3D SWP_DEVICE_EXCLUSIVE_WRITE || | ^~~~~~~~~~~~~~~~~~~~~~~~~~ mm/page_table_check.c:192:24: note: each undeclared identifier is reported only once for each function it appears in mm/page_table_check.c:194:1: error: control reaches end of non-void function [-Werror=3Dreturn-type] 194 | } Looks like there is a dependence on CONFIG_DEVICE_PRIVATE. Pasha