public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: David Hildenbrand <david@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Peter Feiner <pfeiner@google.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Cyrill Gorcunov <gorcunov@openvz.org>,
	Pavel Emelyanov <xemul@parallels.com>,
	Jamie Liu <jamieliu@google.com>, Hugh Dickins <hughd@google.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Muchun Song <songmuchun@bytedance.com>,
	Peter Xu <peterx@redhat.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH 4.9-stable -- 5.19-stable] mm/hugetlb: fix hugetlb not supporting softdirty tracking
Date: Mon, 29 Aug 2022 10:13:59 +0200	[thread overview]
Message-ID: <Ywx1R+FhHTNIKdoo@kroah.com> (raw)
In-Reply-To: <20220825143258.36151-1-david@redhat.com>

On Thu, Aug 25, 2022 at 04:32:58PM +0200, David Hildenbrand wrote:
> commit f96f7a40874d7c746680c0b9f57cef2262ae551f upstream.
> 
> Patch series "mm/hugetlb: fix write-fault handling for shared mappings", v2.
> 
> I observed that hugetlb does not support/expect write-faults in shared
> mappings that would have to map the R/O-mapped page writable -- and I
> found two case where we could currently get such faults and would
> erroneously map an anon page into a shared mapping.
> 
> Reproducers part of the patches.
> 
> I propose to backport both fixes to stable trees.  The first fix needs a
> small adjustment.
> 
> This patch (of 2):
> 
> Staring at hugetlb_wp(), one might wonder where all the logic for shared
> mappings is when stumbling over a write-protected page in a shared
> mapping.  In fact, there is none, and so far we thought we could get away
> with that because e.g., mprotect() should always do the right thing and
> map all pages directly writable.
> 
> Looks like we were wrong:
> 
> --------------------------------------------------------------------------
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <errno.h>
>  #include <sys/mman.h>
> 
>  #define HUGETLB_SIZE (2 * 1024 * 1024u)
> 
>  static void clear_softdirty(void)
>  {
>          int fd = open("/proc/self/clear_refs", O_WRONLY);
>          const char *ctrl = "4";
>          int ret;
> 
>          if (fd < 0) {
>                  fprintf(stderr, "open(clear_refs) failed\n");
>                  exit(1);
>          }
>          ret = write(fd, ctrl, strlen(ctrl));
>          if (ret != strlen(ctrl)) {
>                  fprintf(stderr, "write(clear_refs) failed\n");
>                  exit(1);
>          }
>          close(fd);
>  }
> 
>  int main(int argc, char **argv)
>  {
>          char *map;
>          int fd;
> 
>          fd = open("/dev/hugepages/tmp", O_RDWR | O_CREAT);
>          if (!fd) {
>                  fprintf(stderr, "open() failed\n");
>                  return -errno;
>          }
>          if (ftruncate(fd, HUGETLB_SIZE)) {
>                  fprintf(stderr, "ftruncate() failed\n");
>                  return -errno;
>          }
> 
>          map = mmap(NULL, HUGETLB_SIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
>          if (map == MAP_FAILED) {
>                  fprintf(stderr, "mmap() failed\n");
>                  return -errno;
>          }
> 
>          *map = 0;
> 
>          if (mprotect(map, HUGETLB_SIZE, PROT_READ)) {
>                  fprintf(stderr, "mmprotect() failed\n");
>                  return -errno;
>          }
> 
>          clear_softdirty();
> 
>          if (mprotect(map, HUGETLB_SIZE, PROT_READ|PROT_WRITE)) {
>                  fprintf(stderr, "mmprotect() failed\n");
>                  return -errno;
>          }
> 
>          *map = 0;
> 
>          return 0;
>  }
> --------------------------------------------------------------------------
> 
> Above test fails with SIGBUS when there is only a single free hugetlb page.
>  # echo 1 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages
>  # ./test
>  Bus error (core dumped)
> 
> And worse, with sufficient free hugetlb pages it will map an anonymous page
> into a shared mapping, for example, messing up accounting during unmap
> and breaking MAP_SHARED semantics:
>  # echo 2 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages
>  # ./test
>  # cat /proc/meminfo | grep HugePages_
>  HugePages_Total:       2
>  HugePages_Free:        1
>  HugePages_Rsvd:    18446744073709551615
>  HugePages_Surp:        0
> 
> Reason in this particular case is that vma_wants_writenotify() will
> return "true", removing VM_SHARED in vma_set_page_prot() to map pages
> write-protected. Let's teach vma_wants_writenotify() that hugetlb does not
> support softdirty tracking.
> 
> Link: https://lkml.kernel.org/r/20220811103435.188481-1-david@redhat.com
> Link: https://lkml.kernel.org/r/20220811103435.188481-2-david@redhat.com
> Fixes: 64e455079e1b ("mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Peter Feiner <pfeiner@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Jamie Liu <jamieliu@google.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Muchun Song <songmuchun@bytedance.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: <stable@vger.kernel.org>	[3.18+]
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  mm/mmap.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Now queued up, thanks.

greg k-h


      reply	other threads:[~2022-08-29  8:14 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1661424546448@kroah.com>
2022-08-25 14:32 ` [PATCH 4.9-stable -- 5.19-stable] mm/hugetlb: fix hugetlb not supporting softdirty tracking David Hildenbrand
2022-08-29  8:13   ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ywx1R+FhHTNIKdoo@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=bhelgaas@google.com \
    --cc=david@redhat.com \
    --cc=gorcunov@openvz.org \
    --cc=hughd@google.com \
    --cc=jamieliu@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=peterx@redhat.com \
    --cc=pfeiner@google.com \
    --cc=songmuchun@bytedance.com \
    --cc=stable@vger.kernel.org \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox