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 0D03BC433FE for ; Mon, 21 Nov 2022 21:17:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2F4016B0072; Mon, 21 Nov 2022 16:17:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 27BBC6B0073; Mon, 21 Nov 2022 16:17:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0F5B56B0074; Mon, 21 Nov 2022 16:17:24 -0500 (EST) 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 ED2246B0072 for ; Mon, 21 Nov 2022 16:17:23 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id B425F160BEB for ; Mon, 21 Nov 2022 21:17:23 +0000 (UTC) X-FDA: 80158710366.13.B07E425 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf05.hostedemail.com (Postfix) with ESMTP id 4D0D5100009 for ; Mon, 21 Nov 2022 21:17:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669065442; 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: in-reply-to:in-reply-to:references:references; bh=m1VEqd8Y4SFqXLnlYYQALR4+fdttqcWq0/uqwM6HTCA=; b=IRLmvoH3A7hVTq0El6rLgTOTB+fk4/w0BKIVfPVO0vGLZj5g3MickBSrxvfzLC7gMupgiC Bp3wJFrVXGV+Tpm9XyipYg2kDLZIIPAepjuuPyKk5TtjV/v2AfT/0NoMYaltNFOXBmTIrR U06ISDzW29lXJ0TbDyPMUfiyn0cPPPY= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-590-AHv6apviO9-1JfVSChqD4g-1; Mon, 21 Nov 2022 16:17:14 -0500 X-MC-Unique: AHv6apviO9-1JfVSChqD4g-1 Received: by mail-qt1-f198.google.com with SMTP id w27-20020a05622a191b00b003a56c0e1cd0so12879010qtc.4 for ; Mon, 21 Nov 2022 13:17:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=m1VEqd8Y4SFqXLnlYYQALR4+fdttqcWq0/uqwM6HTCA=; b=3Mt23q0kJ7MWyAmLnjDgailEZesvGZ2xsrT4dXhXnyLwBbWYY0ANlKB/WMh8EtBxuF OrJPgRQytIdJbJumkTjHCJMsdboBwnj/h7OrhGCwhOReudBlnpXbvzr0Iv6R1Eeb2g98 PYjVU+3GzuQ7ODmC7FN5RttCTkL/QaAAN2dtjl02i4I9ahZ4ga/RSy/nptEMkdUsW0DV HWryX688BiA0vVIp12GIguAws40K7HX5A6/iZS1G28mCQOhPETf4+sKQEqZvnGPMMRBx hqIQN0oT2EoiN2ypxTzJf2ODwMwsCHFBccXjksTLfbU6ji5XhvUGL7LBshWyTRrqUpID WIYA== X-Gm-Message-State: ANoB5pkZhoCk57m1H2qY+3+p82CCmxjN+6/Gs3KHZLbi1gWQRberhu/8 9C5TV2oFOY1JLRcYroMGmBMKiRZLqPCUnUttBL2OYy3qlhHk+H0lxemHlon8SdpqOJvoxOD5TU0 KxoRAbI5L8m8= X-Received: by 2002:a05:620a:a03:b0:6fa:3f27:c1e5 with SMTP id i3-20020a05620a0a0300b006fa3f27c1e5mr17534569qka.447.1669065433512; Mon, 21 Nov 2022 13:17:13 -0800 (PST) X-Google-Smtp-Source: AA0mqf6Z8fvyFzBAqWZMgsrv+dhtNnpC0JlC8ob+XIiAqveoOcQx2bjR42VDsoMPeI/zF9aAo4Bj3A== X-Received: by 2002:a05:620a:a03:b0:6fa:3f27:c1e5 with SMTP id i3-20020a05620a0a0300b006fa3f27c1e5mr17534553qka.447.1669065433252; Mon, 21 Nov 2022 13:17:13 -0800 (PST) Received: from x1n (bras-base-aurron9127w-grc-46-70-31-27-79.dsl.bell.ca. [70.31.27.79]) by smtp.gmail.com with ESMTPSA id i9-20020ac813c9000000b003a4c3c4d2d4sm7242891qtj.49.2022.11.21.13.17.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Nov 2022 13:17:12 -0800 (PST) Date: Mon, 21 Nov 2022 16:17:11 -0500 From: Peter Xu To: Muhammad Usama Anjum Cc: David Hildenbrand , Nadav Amit , Andrea Arcangeli , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v4 1/3] mm/mprotect: Fix soft-dirty check in can_change_pte_writable() Message-ID: References: <20220725142048.30450-1-peterx@redhat.com> <20220725142048.30450-2-peterx@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669065443; a=rsa-sha256; cv=none; b=KgPhTwnJNj+UGgw5RvbkM7eA1pUXN9k+iYDq5Eb46efwkgH0Xgkxk1yBb0pRgucjUb0zJT EjD5MZH+dWW2rCcVLO8ohLyVxM8vvKHCm2nUrf62423CLHqfWk5M/uBno8j4eoR+A4l3WK jtRi/RVDxogPV9TZmoLZVrI347Ds1sI= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IRLmvoH3; spf=pass (imf05.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669065443; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=m1VEqd8Y4SFqXLnlYYQALR4+fdttqcWq0/uqwM6HTCA=; b=qSeh0ldue1uk07gV4al/c4ar6lwq/5liDICylmSyNVTZ+4I7YHW0dGCtmjP/B3Mk4AQj6b 9DxjJmw9k5h0XGNN47YR6E5C8Ehik6x2EMfitRTi1dqBJqwwJjxCpqblhkKoi/J0aIAmw/ IT+39E4IhylNvCo0gVqLQu7bEjeBVrQ= X-Stat-Signature: w73ixydpgaaj1b8uq95hn5ge8rxmy1br X-Rspamd-Queue-Id: 4D0D5100009 X-Rspam-User: Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IRLmvoH3; spf=pass (imf05.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspamd-Server: rspam02 X-HE-Tag: 1669065443-727866 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: On Mon, Nov 21, 2022 at 07:57:05PM +0500, Muhammad Usama Anjum wrote: > Hi Peter, > > Thank you so much for replying. > > On 11/19/22 4:14 AM, Peter Xu wrote: > > On Sat, Nov 19, 2022 at 01:16:26AM +0500, Muhammad Usama Anjum wrote: > >> Hi Peter and David, > > > > Hi, Muhammad, > > > >> > >> On 7/25/22 7:20 PM, Peter Xu wrote: > >>> 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. > >> [...] > >>> +static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma) > >>> +{ > >>> + /* > >>> + * NOTE: we must check this before VM_SOFTDIRTY on soft-dirty > >>> + * enablements, because when without soft-dirty being compiled in, > >>> + * VM_SOFTDIRTY is defined as 0x0, then !(vm_flags & VM_SOFTDIRTY) > >>> + * will be constantly true. > >>> + */ > >>> + if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY)) > >>> + return false; > >>> + > >>> + /* > >>> + * Soft-dirty is kind of special: its tracking is enabled when the > >>> + * vma flags not set. > >>> + */ > >>> + return !(vma->vm_flags & VM_SOFTDIRTY); > >>> +} > >> I'm sorry. I'm unable to understand the inversion here. > >>> its tracking is enabled when the vma flags not set. > >> VM_SOFTDIRTY is set on the VMA when new VMA is allocated to mark is > >> soft-dirty. When we write to clear_refs to clear soft-dirty bit, > >> VM_SOFTDIRTY is cleared from the VMA as well. Then why do you say tracking > >> is enabled when the vma flags not set? > > > > Because only when 4>clear_refs happens would VM_SOFTDIRTY be cleared, and > > only until then the real tracking starts (by removing write bits on ptes). > But even if the VM_SOFTDIRTY is set on the VMA, the individual pages are > still marked soft-dirty. Both are independent. > > It means tracking is enabled all the time in individual pages. IMHO it depends on how we define "tracking enabled" - before clear_refs even if no pages written they'll also be reported as dirty, then the information is useless. > Only the soft-dirty bit status in individual page isn't significant if > VM_SOFTDIRTY already is set. Right? Yes. But I'd say it makes more sense to say "tracking enabled" if we really started tracking (by removing the write bits in ptes, otherwise we did nothing). When vma created we didn't track anything. I don't know the rational of why soft-dirty was defined like that. I think it's somehow related to the fact that we allow false positive dirty pages not false negative. IOW, it's a bug to leak a page being dirtied, but not vice versa if we report clean page dirty. -- Peter Xu