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 X-Spam-Level: X-Spam-Status: No, score=-22.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF308C3F68F for ; Fri, 14 Feb 2020 22:59:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8613320848 for ; Fri, 14 Feb 2020 22:59:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="V1jpe9t3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8613320848 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 361486B0003; Fri, 14 Feb 2020 17:59:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3134D6B0005; Fri, 14 Feb 2020 17:59:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 202DC6B0006; Fri, 14 Feb 2020 17:59:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0033.hostedemail.com [216.40.44.33]) by kanga.kvack.org (Postfix) with ESMTP id 060656B0003 for ; Fri, 14 Feb 2020 17:59:14 -0500 (EST) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 9CE4A2463 for ; Fri, 14 Feb 2020 22:59:13 +0000 (UTC) X-FDA: 76490250186.16.mass62_164b9dd33290f X-HE-Tag: mass62_164b9dd33290f X-Filterd-Recvd-Size: 5855 Received: from mail-pj1-f74.google.com (mail-pj1-f74.google.com [209.85.216.74]) by imf36.hostedemail.com (Postfix) with ESMTP for ; Fri, 14 Feb 2020 22:59:13 +0000 (UTC) Received: by mail-pj1-f74.google.com with SMTP id ds13so6767446pjb.6 for ; Fri, 14 Feb 2020 14:59:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:message-id:mime-version:subject:from:to:cc; bh=159AKCujg+gW8fqSsGG8pDd0kcHsLFha/U7wQ4KOefk=; b=V1jpe9t30LEtuFBkzCUikejPz9SfFkrsw2gy0kqScijtgPMOXDn33Y6Y22NHzuKX57 HQnn5udcQ+qlzZ1EIWvpuK3MTgrepegBJ5Wx8X0H+wE0p3WKF0ENmV7mLsmknDnReWxH B4cPmmUkSAcJgrcq+pH2Glgh0Yt+DvmnrdZYlyEA6oE7u6mhF8OCE5o75mfe/cokvob5 2CEKhY4j4gFtDaAO+70TJuxVko90bzS8hmZYwDtL60CPCykRj10ZEEXx9wl19ekFBD/N DJ7b0WnKMSc1UMrhmrMN+MJSv02T7gkpCm1IvdzORao60lp3Vg8F0uozjsukknUTRG4W gS6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=159AKCujg+gW8fqSsGG8pDd0kcHsLFha/U7wQ4KOefk=; b=PNnKLsC0fsok2y7JbtxRF9GgdFGMnrbFawj1vWeG7rx11TskabrLfwbp0w+9m1ROcB NO92Sug1gDVK8rZYCG1Da9lQVNn/fX9c1XDG6btcbY08R/i7hy90HigxgHuzC0qP8VU6 QFe+P7SVU5se+tppaOtrnwgEZVDL7E7AiXnJKNvFIp/do/M88yOcMno5f29PqUKxQ1Cd Vr0JNY6dDmzug54ogaQ3V8B6yRdMLLnxCNMyoOnDlZuAfhqwL1G0pFXuZEsyH8rWKaD7 Gz2Y5zrSciTLTNBW9bByTL9nKKrkSiLVskH5iCuhmbT4x/f20gIbuyp1Ts5AY52W/k7+ wMRQ== X-Gm-Message-State: APjAAAXlcysljAERVSxhc2YeptsqagLVDZM7WzHCQ3EM1/gRYOHEUTJA 6fKgS5yCyCkOvFfcPnMkgzfyQxBlYHNZ X-Google-Smtp-Source: APXvYqzwCvOZWCeRS1PqBNuUe+OPc2VKkaWEfaI//TYEzoih7GC0pWCFmsyzy+76Weu1vHGsOCQkFWgRklMx X-Received: by 2002:a63:4281:: with SMTP id p123mr5723142pga.371.1581721151869; Fri, 14 Feb 2020 14:59:11 -0800 (PST) Date: Fri, 14 Feb 2020 14:58:49 -0800 Message-Id: <20200214225849.108108-1-bgeffon@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.25.0.265.gbab2e86ba0-goog Subject: [RFC PATCH] userfaultfd: Address race after fault. From: Brian Geffon To: Andrew Morton Cc: linux-mm , linux-kernel@vger.kernel.org, Andrea Arcangeli , Mike Rapoport , Sonny Rao , "Kirill A . Shutemov" , Brian Geffon Content-Type: text/plain; charset="UTF-8" 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 faulting path is: do_user_addr_fault (flags = FAULT_FLAG_ALLOW_RETRY) handle_mm_fault __handle_mm_fault do_anonymous handle_userfault (ret = VM_FAULT_RETRY) At this point the fault is handled and when this call sequence unwinds it is expected that the PTEs are present as handle_userfault took care of that and returned VM_FAULT_RETRY. When we unwind back down to do_user_addr_fault it will attempt to retry after clearing FAULT_FLAG_ALLOW_RETRY and setting FAULT_FLAG_TRIED. At any point between the fault being handle and when it's retried a userspace thread was to zap the page range, let's say via madvise(MADV_DONTNEED). Now as this fault is being retried the PTEs would be missing again and we land right back in handle_userfault. Although this time since FAULT_FLAG_ALLOW_RETRY has been cleared we're going to bail and return VM_FAULT_SIGBUS. This scenario is easy to reproduce and I observed it while writing tests for MREMAP_DONTUNMAP in the userfaultfd selftests. I have a standalone example of this that uses madvise(MADV_DONTNEED) to cause this race: https://gist.github.com/bgaff/3a8b2a890ae4771be22456e014c2e5aa Given that this is genuinely a new fault, is a SIGBUS the best way to go about this? Since userfaultfd is designed to be used in a non-cooperative case, could it be more resilient and instead retry for the new fault? With that being said for VM_UFFD_MISSING userfaults can we just remove the check in handle_userfault that FAULT_FLAG_ALLOW_RETRY is set in vmf->flags and allow it to retry the fault to address this race scenario? In my testing that does solve the problem, but does it possibly create others? Is this the best way to go about it? I'll defer to the domain experts for any recommendations here in case I'm way off. Thanks for your time. Signed-off-by: Brian Geffon --- fs/userfaultfd.c | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index ebf17d7e1093..6407fec798ff 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -416,34 +416,6 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) goto out; } - /* - * Check that we can return VM_FAULT_RETRY. - * - * NOTE: it should become possible to return VM_FAULT_RETRY - * even if FAULT_FLAG_TRIED is set without leading to gup() - * -EBUSY failures, if the userfaultfd is to be extended for - * VM_UFFD_WP tracking and we intend to arm the userfault - * without first stopping userland access to the memory. For - * VM_UFFD_MISSING userfaults this is enough for now. - */ - if (unlikely(!(vmf->flags & FAULT_FLAG_ALLOW_RETRY))) { - /* - * Validate the invariant that nowait must allow retry - * to be sure not to return SIGBUS erroneously on - * nowait invocations. - */ - BUG_ON(vmf->flags & FAULT_FLAG_RETRY_NOWAIT); -#ifdef CONFIG_DEBUG_VM - if (printk_ratelimit()) { - printk(KERN_WARNING - "FAULT_FLAG_ALLOW_RETRY missing %x\n", - vmf->flags); - dump_stack(); - } -#endif - goto out; - } - /* * Handle nowait, not much to do other than tell it to retry * and wait. -- 2.25.0.265.gbab2e86ba0-goog