From patchwork Wed Jul 20 22:03:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Xu X-Patchwork-Id: 12924537 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 825BAC433EF for ; Wed, 20 Jul 2022 22:03:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F01036B0073; Wed, 20 Jul 2022 18:03:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EAF336B0074; Wed, 20 Jul 2022 18:03:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D51AC6B0075; Wed, 20 Jul 2022 18:03:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id C3AE06B0073 for ; Wed, 20 Jul 2022 18:03:30 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9F73AAA830 for ; Wed, 20 Jul 2022 22:03:30 +0000 (UTC) X-FDA: 79708855380.29.AF25617 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf19.hostedemail.com (Postfix) with ESMTP id 1C1181A0087 for ; Wed, 20 Jul 2022 22:03:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1658354609; 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; bh=x1PqLLBnq81fcVYE6PRnR8ilrAhon4mRiyv85by7udw=; b=U5PXZrJNG6o2MXYvhTldOyLy1dCqZMO7uWwgt86gfUkp6KIejRE15p1Tzku9qXgXjInGHU +Rso/xsvZNRhwfLAAQd7nX+SPWNJQ6rS/QsyhZYlLok65wX55repG8hf23AHXJyjc0he3U wNcVFUlJK8j2I2xajfgsbVeB7wwFowI= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-290-joyliztvOWOvwyJ1JnFNOg-1; Wed, 20 Jul 2022 18:03:26 -0400 X-MC-Unique: joyliztvOWOvwyJ1JnFNOg-1 Received: by mail-qk1-f198.google.com with SMTP id ay35-20020a05620a17a300b006b5d9646d31so61245qkb.6 for ; Wed, 20 Jul 2022 15:03:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=x1PqLLBnq81fcVYE6PRnR8ilrAhon4mRiyv85by7udw=; b=nPtueRSoobcRkEmRk+Xb1UTs6/lE86couehZxPM48JoyLTrmgZEz2cGMJ2JfUyAWUm S5accPKwGkAMv+8sItjauM1pLWqj+/HUZxVr8o7EYr+fabm7SxiWQ7BEGtO7u5jeQgvD P5Edxb7jSrIUwt2/mzsKBnCNSUXC27mwhD0Y0fN9aiAtVKY7rEeGWZ6CnpKPVycsPDea 7YcFf+uCvRSE+UAizmkitXzdp51XujyTiuAEKxW6umwtRe89rXJE2yfVdJBYR7bYJJnP mr9PPa+s63CVK/8kw+pdE5WerRcL0oh1ohZWySm7HcbswZQKLWJ3Iv/KlI3yJVQl9gxx 3tjQ== X-Gm-Message-State: AJIora9w/i/K8Er/CknQ86IIlleHoY0xOvQu8YII0CgVPplv8tK5DpFB tGPYbyYpVY4mYXx7wf2CJxOd6XpZrM1uPlx9XbKchYS5bxY1qTuEFoQxlkvGTHfg8GTSvhpQMdZ NihAYlbvGtyc= X-Received: by 2002:a05:620a:1907:b0:6a6:2fac:462f with SMTP id bj7-20020a05620a190700b006a62fac462fmr25106197qkb.213.1658354606064; Wed, 20 Jul 2022 15:03:26 -0700 (PDT) X-Google-Smtp-Source: AGRyM1si1BGAVziAsccaGFKsgWpAnO0r1FpskyjFUTXIZYMAWYfTfMf5Ez4uvNHyWyhyJ/EOwHK1Cw== X-Received: by 2002:a05:620a:1907:b0:6a6:2fac:462f with SMTP id bj7-20020a05620a190700b006a62fac462fmr25106174qkb.213.1658354605757; Wed, 20 Jul 2022 15:03:25 -0700 (PDT) Received: from localhost.localdomain (bras-base-aurron9127w-grc-37-74-12-30-48.dsl.bell.ca. [74.12.30.48]) by smtp.gmail.com with ESMTPSA id x2-20020a05620a448200b006a8b6848556sm262432qkp.7.2022.07.20.15.03.24 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Wed, 20 Jul 2022 15:03:25 -0700 (PDT) From: Peter Xu To: linux-kernel@vger.kernel.org, linux-mm@kvack.org Cc: Andrea Arcangeli , peterx@redhat.com, Nadav Amit , David Hildenbrand , Andrew Morton Subject: [PATCH v2] mm/mprotect: Fix soft-dirty check in can_change_pte_writable() Date: Wed, 20 Jul 2022 18:03:24 -0400 Message-Id: <20220720220324.88538-1-peterx@redhat.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-type: text/plain ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=U5PXZrJN; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf19.hostedemail.com: domain of peterx@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658354610; a=rsa-sha256; cv=none; b=fni1BuGElpN2QLrCru4iYtCQYFEjPVUSkBUPsfq/tKNCWyZ7ziAMYv12Dyu8Sa5l78+Kd8 AwqoQw2P+EZhw7+B7hRQh5n6bCTlCFgfvyeGdMxytIHyPI/64/GBrtEUhJ09yC837MupOr OOrkt0eIdAVNsGvH0qIDISBuJLu4G3U= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658354610; 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: references:dkim-signature; bh=x1PqLLBnq81fcVYE6PRnR8ilrAhon4mRiyv85by7udw=; b=XZ1l0bvnpOgOfVBTCo1iFXREAQ42qADO578KOlKl9glwHNn9INpuoOB9HlbdglpLR2JjYR vyu/HDy9tAEWQ8s66yDzM38u6qt9AFobGeg/dZDByX8pFw2spJS5Ulp/JblwMZS3Z4x2Qs qJ9PVqe21svmLVdSrYZxQdBhvoxdl80= X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 1C1181A0087 Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=U5PXZrJN; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf19.hostedemail.com: domain of peterx@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=peterx@redhat.com X-Stat-Signature: n5ea7mncuaq59upxwmznu3mg4z5pekj4 X-HE-Tag: 1658354609-973824 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: The check wanted to make sure when soft-dirty tracking is enabled we won't grant write bit by accident, as a page fault is needed for dirty tracking. The intention is correct but we didn't check it right because VM_SOFTDIRTY set actually means soft-dirty tracking disabled. Fix it. It wasn't a bug for a long time because we used to only optimize the write bit settings in change_pte_range() for page caches, and since we've got a higher level check in vma_wants_writenotify(), we will never set the bit MM_CP_TRY_CHANGE_WRITABLE for soft-dirty enabled page caches, hence even if we checked with the wrong value of VM_SOFTDIRTY in change_pte_range() it'll just be an no-op. Functionally it was still correct, even if cpu cycles wasted. However after the recent work of anonymous page optimization on exclusive pages we'll start to make it wrong because anonymous page does not require the check in vma_wants_writenotify() hence it'll suffer from the wrong check here in can_change_pte_writable(). We can easily verify this with any exclusive anonymous page, like program below: =======8<====== #include #include #include #include #include #include #include #include #include #include #include #include #include #define BIT_ULL(nr) (1ULL << (nr)) #define PM_SOFT_DIRTY BIT_ULL(55) unsigned int psize; char *page; uint64_t pagemap_read_vaddr(int fd, void *vaddr) { uint64_t value; int ret; ret = pread(fd, &value, sizeof(uint64_t), ((uint64_t)vaddr >> 12) * sizeof(uint64_t)); assert(ret == sizeof(uint64_t)); return value; } void clear_refs_write(void) { int fd = open("/proc/self/clear_refs", O_RDWR); assert(fd >= 0); write(fd, "4", 2); close(fd); } #define check_soft_dirty(str, expect) do { \ bool dirty = pagemap_read_vaddr(fd, page) & PM_SOFT_DIRTY; \ if (dirty != expect) { \ printf("ERROR: %s, soft-dirty=%d (expect: %d)\n", str, dirty, expect); \ exit(-1); \ } \ } while (0) int main(void) { int fd = open("/proc/self/pagemap", O_RDONLY); assert(fd >= 0); psize = getpagesize(); page = mmap(NULL, psize, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); assert(page != MAP_FAILED); *page = 1; check_soft_dirty("Just faulted in page", 1); clear_refs_write(); check_soft_dirty("Clear_refs written", 0); mprotect(page, psize, PROT_READ); check_soft_dirty("Marked RO", 0); mprotect(page, psize, PROT_READ|PROT_WRITE); check_soft_dirty("Marked RW", 0); *page = 2; check_soft_dirty("Wrote page again", 1); munmap(page, psize); close(fd); printf("Test passed.\n"); return 0; } =======8<====== So even if commit 64fe24a3e05e kept the old behavior and didn't attempt to change the behavior here, the bug will only be able to be triggered after commit 64fe24a3e05e because only anonymous page will suffer from it. Fixes: 64fe24a3e05e ("mm/mprotect: try avoiding write faults for exclusive anonymous pages when changing protection") Signed-off-by: Peter Xu --- mm/mprotect.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/mprotect.c b/mm/mprotect.c index 0420c3ed936c..804807ab14e6 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -48,8 +48,11 @@ static inline bool can_change_pte_writable(struct vm_area_struct *vma, if (pte_protnone(pte) || !pte_dirty(pte)) return false; - /* Do we need write faults for softdirty tracking? */ - if ((vma->vm_flags & VM_SOFTDIRTY) && !pte_soft_dirty(pte)) + /* + * Do we need write faults for softdirty tracking? Note, + * soft-dirty is enabled when !VM_SOFTDIRTY. + */ + if (!(vma->vm_flags & VM_SOFTDIRTY) && !pte_soft_dirty(pte)) return false; /* Do we need write faults for uffd-wp tracking? */