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 25AECC4345F for ; Wed, 17 Apr 2024 19:45:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A36CA6B0083; Wed, 17 Apr 2024 15:45:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9BFAA6B0088; Wed, 17 Apr 2024 15:45:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 839086B009A; Wed, 17 Apr 2024 15:45:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 5FF466B0083 for ; Wed, 17 Apr 2024 15:45:09 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 07EB740507 for ; Wed, 17 Apr 2024 19:45:09 +0000 (UTC) X-FDA: 82020052338.05.2D1F179 Received: from mail-qt1-f179.google.com (mail-qt1-f179.google.com [209.85.160.179]) by imf11.hostedemail.com (Postfix) with ESMTP id 156B640016 for ; Wed, 17 Apr 2024 19:45:06 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=soleen-com.20230601.gappssmtp.com header.s=20230601 header.b=r2RZOafN; spf=pass (imf11.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.160.179 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=1713383107; 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=4kEFUbyjFnPYj78HJaBv20WJ3deP8o9MXIlptLFC9gA=; b=3VSWkhe/LYtTpO9QZt7UrNs6HbItWeX0KAYoU2WM4zNL/z6odq65riy2u3uH6RfPMjopez dejTatY4GM6nevtAGMe8rcKxp4kZEZOPUBxLMhKsoFWxDmX/xK4lpgnz34F+AllQ5Rb+ob lIqspdlw0JQpwsJwqgW/Yfdle9DqQN0= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=soleen-com.20230601.gappssmtp.com header.s=20230601 header.b=r2RZOafN; spf=pass (imf11.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.160.179 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=1713383107; a=rsa-sha256; cv=none; b=2AcFTVv5r6eMm8DWzrE3s/UdlPZxxHsI03PWiQTSuCxLjgw9qzHh2Yvyg/3ddvxTS814TV UPYJNaIBwgijCAXAit5/bPe8DmUVZyDLKLo/jl92E/C/NpnDygn9ag0gZR2lKDwp+WRKER AJQKVLmu7ME5gVdUnlDD730R0BVGV+o= Received: by mail-qt1-f179.google.com with SMTP id d75a77b69052e-434b7ab085fso9813711cf.1 for ; Wed, 17 Apr 2024 12:45:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=soleen-com.20230601.gappssmtp.com; s=20230601; t=1713383106; x=1713987906; 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=4kEFUbyjFnPYj78HJaBv20WJ3deP8o9MXIlptLFC9gA=; b=r2RZOafNQz6/jZgUDmUY39ObiQ4T5YDbL5+MsSlq4UFFpAVueuduAFutUozD8SjEd4 4ia0ePildlugYcHM1vlq2Jyz7hDJpL06Rob9PTQRhfLpUrHmE0xJ3HBfZOwVC1go08UV 3OSRfzVXc41ykzFoK0yYM1mQKwAXsqILy6MT0dZLnNqOtY8TrdIhz3OA6SzTAWPf4B+K oILswMknST1u0MFe3p3frzF2iFvOFtI+FbTJiKkSzIqhJToS8aBkpPRNoARRVWgkK4T7 B9NdKbClR9XCIiHuBabsi6Jbn3r1HQtGMBn9Jk7KaHmlkn0PpmSBjq6zOKOWt3DBd1A2 dKIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713383106; x=1713987906; 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=4kEFUbyjFnPYj78HJaBv20WJ3deP8o9MXIlptLFC9gA=; b=xP8h3RSJlg2kS042vG1FoB4ekdOxxR0NJ8R28VuNajz+rETMLFb5+/EiidLOUkUItr 7aXiv3bUN/gg1ea8ZChPdSeEtM2qp/mf01wLG/z5ASFc0sjnV81MAMJW+owEOjiCpBK4 Gej4SpUcBeb+RAaAR3QLqh3pYkYjlMKQFqDD5EUiP0dIWxNddZibAiX7OrLuUPLUJGOT D0CVTYGdhYAzoZkQLcF8H/h/5KpX6dnxEzwqlEeNPWirfvB5ayODAaFCTkbJwSqpKUux WqABdMnt8ETRjvYQRoTz6eq1LYv6DZknM2tvid34u5Q6aSqsGQMU4I43yKhGfmM8z3G/ am6w== X-Forwarded-Encrypted: i=1; AJvYcCUVxQE94fOGzZ/HYvg2cDoHTBkj/caUni4jRJRaPClrBRFXKV+VzSDwTMH6f4t1dKsZSuMfrJRxMgwocZGMR7NdHNM= X-Gm-Message-State: AOJu0YzJhwsaXZPdycsk5wOOyrA+dieIyxV5VfKKTXN0AJR7YLsn0cQu 7MnKSebu6Opv1rGS6w5KCUbaS4TgGO8WCx0ahKYs32iFIv7f2NXkQSNKiLAl6A9OYCUDzhe2DLL 0T2lgRcBvM3AyWy26EjxMfcurLX4q6VUJ2oOOTA== X-Google-Smtp-Source: AGHT+IF9BhHzehlb5Ie9FDkZYWNsAe4CGi4O39HYr6ZO33yCJ0uRwYckXrJ406izlSTAs1xJqgFG25ohOhGQZVFSDzY= X-Received: by 2002:ac8:7dcf:0:b0:434:6ff9:266c with SMTP id c15-20020ac87dcf000000b004346ff9266cmr199512qte.8.1713383106156; Wed, 17 Apr 2024 12:45:06 -0700 (PDT) MIME-Version: 1.0 References: <20240417192643.2671335-1-peterx@redhat.com> In-Reply-To: <20240417192643.2671335-1-peterx@redhat.com> From: Pasha Tatashin Date: Wed, 17 Apr 2024 15:44:29 -0400 Message-ID: Subject: Re: [PATCH v3] mm/page_table_check: Support userfault wr-protect entries To: Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , David Hildenbrand , Nadav Amit , Axel Rasmussen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 156B640016 X-Stat-Signature: hae65j911uw6hzjczunsmd9nduofu5mg X-Rspam-User: X-Rspamd-Server: rspam07 X-HE-Tag: 1713383106-239193 X-HE-Meta: U2FsdGVkX1+T9e5hJjZ1Q0gMUYlYf56zXHYw17ZVwaite/44s9OcztWkdx9Twqy4uA5XYSSBZCQqueyLNuEN3pHNOm//NHPwwzKmpU4171gf/b3FP9aZrnsdvR4EdgD3GGC06IA4VL9+jzr/reNtLRG2r78ix3DXBKkfhxAyahzRbiZeftVNqvF3a9P0sXYVMNV42L97J0ie8hi10GhcZqpDXz9qllxybQFj0SBXUDBGMDROQZ9dLxKOIs99VwDYs0w8t95icBQAkTa5Q1djmKLvghCq9Y7WXYiSMARdB4uM0dhPG/4QJxoJIBqMUoQn4NqpQyFrPMCv1Gieq9y3RJuo+TPTM29HYN4bVCGNzFRUSBIN7eXz38E1/L6KoNz6uZ5kUv/sR808w1kuyZ1U275mQ4mxoa6ufQgOGD+NzJX6tglUBD5FDWz4hEh7m1MBjqdSLglBCVgzpDAxRTMkrLp3NtfQC86ilJvJVmPJ9fB+hm9AgnhH+gPz2YcBdW4LaVid3LFa7CQpCAqI6/x5l+kz0uaHLqW+vKzdYNo+azynNRaRlft9G01To/2JNQo0yVK5R8qlJ9KlaeFCND1sIT+daGI6FfqKq3PXMSJQV0qLhjglva7E5c8jgojqcV1XWRXf8qUd9Da6d93zVRHL/DVPoP6B5FWodxdjZsjTtH4cJ3PQqwz/I1nEepSuttyDi5LcDowRYD5bVicj5NeL4QL+/vko2PMXjZFCLmvCzna4utTVfkHzaCwZhGveue9snl7+CrfRaUvePeaWQMaOxyQcuzPMhDyuhVEZfiblDriI1eVXF1s5r5yGQXRK0DGxHillY0t37hwo1CYea8T/9vvMJbsFI7UgsP2HGfx4X6r+s1+4EhQQRQE4iqUNCVFZzpzPsr+6Wmv/BSSa54QLFiQnmhljnR1scl2jzfREH/jxlErdQAOqPcFPSb4dI5xXb0eoeo/gT2SMYWLKrRj HNld7Rsh xlcMZeuVKK9AeJdIw02YIJ3v2WXDrew/g4hQ1EJ97Ci4E5Gtea6snpypjd1Gy+ZmiEpEmwwis5swWthQKg/Pk87xD7qyZaW1VdkeEPSLh8pg0ZlZbd7dCaqFY8zlZmDRKh21EkEBS0icGaGIPmzgxnq0Ta5ncgzwHzxLTY1OnMeYQ0ZfK/GIXBeWWQcLRHuNlV2TsDJoBvI0Dzs8lgqRHp+Zt6DfqHknBu9uAaIFBr2C1SJoPml23lhyvIfsImQEAdHBU7Uh+mqTjY+JE/9v4LPCaLU0pYtB3ZVfF88y1nLnuVY7J/w6xOsCKDTpOXMofn3wk9andVkdvmo8JbWzD5yEGNXsxVAk2n39Zay/qtkknyqEJFvnTq2qzGTTmizwwDsmhMRDiciEUIr6m1lC6vyXd43MmjDH7SoNoZ2QKmztGNBI/z/DzyFf/p8keVq0QwB1CKu4QzqXxbPdCvHbj+8LhW6Q7GpEjSv/IlCZx7Gz/i/AtWeIzR0N8Ix4dAtlKEMOvAaq/vPN04CUm5y5OO9lwIb1rnldZAcxTOGONYMrgOHM/Myq+GWdj1JLHRR1yA0Ba 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 3:26=E2=80=AFPM Peter Xu wrote: > > Allow page_table_check hooks to check over userfaultfd wr-protect criteri= a > upon pgtable updates. The rule is no co-existance allowed for any writab= le > flag against userfault wr-protect flag. > > This should be better than c2da319c2e, where we used to only sanitize suc= h > 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 recent > 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 confi= gs > and/or boot parameters, but should be good enough to track at least > 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 distros, > while distros also normally not enable PAGE_TABLE_CHECK[_ENFORCED], whi= ch > 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 th= e > rest scenarios, which should be the common cases so should be good > enough. > > - Hugetlb check is a bit hairy, as the page table check cannot identify > hugetlb pte or normal pte via trapping at set_pte_at(), because of the > current design where hugetlb maps every layers to pte_t... For example, > the default set_huge_pte_at() can invoke set_pte_at() directly and lose > the hugetlb context, treating it the same as a normal pte_t. So far 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, because the= n > one huge_pte_uffd_wp() per-arch will stop making sense first.. as of no= w > we can leave this for later too. > > This patch also removes commit c2da319c2e altogether, as we have somethin= g > better now. > > [1] https://lore.kernel.org/all/000000000000dce0530615c89210@google.com/ > > Cc: Pasha Tatashin > Signed-off-by: Peter Xu > --- > v2: > - Rename __page_table_check_pxx() to page_table_check_pxx_flags(), > meanwhile move the pte check out of the loop [Pasha] > - Fix build issues reported from the bot, also added SWP_DEVICE_WRITE whi= ch > was overlooked before > v3: > - Add missing doc update [Pasha] > --- > Documentation/mm/page_table_check.rst | 9 ++++++- > arch/x86/include/asm/pgtable.h | 18 +------------ > mm/page_table_check.c | 39 +++++++++++++++++++++++++++ > 3 files changed, 48 insertions(+), 18 deletions(-) > > diff --git a/Documentation/mm/page_table_check.rst b/Documentation/mm/pag= e_table_check.rst > index c12838ce6b8d..5bd1d987d76d 100644 > --- a/Documentation/mm/page_table_check.rst > +++ b/Documentation/mm/page_table_check.rst > @@ -14,7 +14,7 @@ Page table check performs extra verifications at the ti= me when new pages become > accessible from the userspace by getting their page table entries (PTEs = PMDs > etc.) added into the table. > > -In case of detected corruption, the kernel is crashed. There is a small > +In case of most detected corruption, the kernel is crashed. There is a s= mall > performance and memory overhead associated with the page table check. Th= erefore, > it is disabled by default, but can be optionally enabled on systems wher= e the > extra hardening outweighs the performance costs. Also, because page tabl= e check > @@ -22,6 +22,13 @@ is synchronous, it can help with debugging double map = memory corruption issues, > by crashing kernel at the time wrong mapping occurs instead of later whi= ch is > often the case with memory corruptions bugs. > > +It can also be used to do page table entry checks over various flags, du= mp > +warnings when illegal combinations of entry flags are detected. Current= ly, > +userfaultfd is the only user of such to sanity check wr-protect bit agai= nst > +any writable flags. Illegal flag combinations will not directly cause d= ata > +corruption in this case immediately, but that will cause read-only data = to > +be writable, causing data corrupt when the page content is later modifie= d. I would replace: "causing data corrupt ..." to "leading to corruption ..." > + > Double mapping detection logic > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D > > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtabl= e.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 fatal, > - * because it means the uffd-wp bit will be ignored and write wil= l > - * 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 setup. > - */ > - 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..388bcf60d8b5 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,31 @@ void __page_table_check_pud_clear(struct mm_struct *= 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); > + > +#ifdef CONFIG_DEVICE_PRIVATE > + if (type =3D=3D SWP_DEVICE_EXCLUSIVE_WRITE || type =3D=3D SWP_DEV= ICE_WRITE) > + return true; > +#endif > +#ifdef CONFIG_MIGRATION > + if (type =3D=3D SWP_MIGRATION_WRITE) > + return true; > +#endif > + > + return false; > +} This should be re-written like this: static inline bool swap_cached_writable(swp_entry_t entry) { return is_writable_device_exclusive_entry(entry) || is_writable_device_private_entry(entry) || is_writable_migration_entry(entry); } Otherwise the patch looks good. Pasha