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=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham 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 857C1C433E0 for ; Mon, 21 Dec 2020 19:01:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0866A22AAC for ; Mon, 21 Dec 2020 19:01:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0866A22AAC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6745F6B005C; Mon, 21 Dec 2020 14:01:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 623F86B005D; Mon, 21 Dec 2020 14:01:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4C4826B0068; Mon, 21 Dec 2020 14:01:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0170.hostedemail.com [216.40.44.170]) by kanga.kvack.org (Postfix) with ESMTP id 3695E6B005C for ; Mon, 21 Dec 2020 14:01:50 -0500 (EST) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id EEC30485C for ; Mon, 21 Dec 2020 19:01:49 +0000 (UTC) X-FDA: 77618208738.01.boats88_170037527459 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id A78551004EF7D for ; Mon, 21 Dec 2020 19:01:49 +0000 (UTC) X-HE-Tag: boats88_170037527459 X-Filterd-Recvd-Size: 6866 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by imf32.hostedemail.com (Postfix) with ESMTP for ; Mon, 21 Dec 2020 19:01:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608577308; 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=NZiUcV7BSK7i0CWlurCWLtNY549184RJIWz+r5r7B5E=; b=CLWFTLhh6y635bZCyalJPYjNt7CrcXaQT0xrZIa83OBUtF+Gfydn0CWvZJiI/IEHjkG6xj SwsEnFecV4g7F9uw7GbFDvA9e1Dbty2KSrqYRSFSMeIUwBTfxbOAUL0WPTUh0JJqPbv8+9 CJvxI91upwv+s6Z9fDtowoT3bGvWR1g= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-381-1wVHJ_u7OMCGX-uKUJTDzA-1; Mon, 21 Dec 2020 14:01:46 -0500 X-MC-Unique: 1wVHJ_u7OMCGX-uKUJTDzA-1 Received: by mail-qt1-f200.google.com with SMTP id w3so8473356qti.17 for ; Mon, 21 Dec 2020 11:01:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=NZiUcV7BSK7i0CWlurCWLtNY549184RJIWz+r5r7B5E=; b=UUbNUVoESJP1oEDT4XwwpF3CPrKaP3zolmztUmiL0ZUCb+CNPME7t6ppoV/KekBpSJ Ntk5AgrmTyh3llZ7j7awuUx57GAeoeVDZ3it4Bv7LC5L7hCZWnmOv8wdS2x0VTUlHDSl SdD2fCHT/0qfWuUJXH+7kw8fficP3sqP81/MI3LDqddYycczK9ZDj6dIYMd6vXzlvuYX 2a/dnT2YyIQY6XFbmI8recuzm9ro5hoCtn/BgSbM/IFkJV7bwlBXwQpHgKCH5/NaP2pe e+UikGNC7aPtYTzw5AGucRUby+bqTeqFcL+vik3U6BR6k7av0GFLPMQ48apbiU/MnRk6 il/A== X-Gm-Message-State: AOAM531U6/FaNLZl/fTLXg4ZA1D67mFx3UEUTRbLNImhTbG7vre84bFQ 4qH2fw3XQNMIT34JPKobe0jd0PqJd4/d/Qb7vNyMa+vBxaPagY44FkeFaw1YX9up62T3N1aXRmc 1MT7w4TtprAo= X-Received: by 2002:a05:622a:110:: with SMTP id u16mr17814130qtw.181.1608577305250; Mon, 21 Dec 2020 11:01:45 -0800 (PST) X-Google-Smtp-Source: ABdhPJwbUgz+dyrK2CDdLmiez7hlLMcNsr37TcWBJHlCy7Hi/1LYq/N8tyN83npjnd8BPX5EUlRlUg== X-Received: by 2002:a05:622a:110:: with SMTP id u16mr17814113qtw.181.1608577304992; Mon, 21 Dec 2020 11:01:44 -0800 (PST) Received: from xz-x1 ([142.126.83.202]) by smtp.gmail.com with ESMTPSA id z20sm11392783qtb.31.2020.12.21.11.01.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Dec 2020 11:01:44 -0800 (PST) Date: Mon, 21 Dec 2020 14:01:42 -0500 From: Peter Xu To: Mike Kravetz Cc: Nadav Amit , linux-fsdevel@vger.kernel.org, Nadav Amit , Jens Axboe , Andrea Arcangeli , Alexander Viro , io-uring@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH 01/13] fs/userfaultfd: fix wrong error code on WP & !VM_MAYWRITE Message-ID: <20201221190142.GG6640@xz-x1> References: <20201129004548.1619714-1-namit@vmware.com> <20201129004548.1619714-2-namit@vmware.com> <3af643ec-b392-617c-cd4e-77db0cba24bd@oracle.com> MIME-Version: 1.0 In-Reply-To: <3af643ec-b392-617c-cd4e-77db0cba24bd@oracle.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=peterx@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline 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 Tue, Dec 01, 2020 at 01:22:32PM -0800, Mike Kravetz wrote: > On 11/28/20 4:45 PM, Nadav Amit wrote: > > From: Nadav Amit > > > > It is possible to get an EINVAL error instead of EPERM if the following > > test vm_flags have VM_UFFD_WP but do not have VM_MAYWRITE, as "ret" is > > overwritten since commit cab350afcbc9 ("userfaultfd: hugetlbfs: allow > > registration of ranges containing huge pages"). > > > > Fix it. > > > > Cc: Mike Kravetz > > Cc: Jens Axboe > > Cc: Andrea Arcangeli > > Cc: Peter Xu > > Cc: Alexander Viro > > Cc: io-uring@vger.kernel.org > > Cc: linux-fsdevel@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-mm@kvack.org > > Fixes: cab350afcbc9 ("userfaultfd: hugetlbfs: allow registration of ranges containing huge pages") > > Signed-off-by: Nadav Amit > > --- > > fs/userfaultfd.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > > index 000b457ad087..c8ed4320370e 100644 > > --- a/fs/userfaultfd.c > > +++ b/fs/userfaultfd.c > > @@ -1364,6 +1364,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, > > if (end & (vma_hpagesize - 1)) > > goto out_unlock; > > } > > + ret = -EPERM; > > if ((vm_flags & VM_UFFD_WP) && !(cur->vm_flags & VM_MAYWRITE)) > > goto out_unlock; > > > > Thanks! We should return EPERM in that case. > > However, the check for VM_UFFD_WP && !VM_MAYWRITE went in after commit > cab350afcbc9. I think it is more accurate to say that the issue was > introduced with commit 63b2d4174c4a ("Introduce the new uffd-wp APIs > for userspace."). The convention in userfaultfd_register() is that the > return code is set before testing condition which could cause return. > Therefore, when 63b2d4174c4a added the VM_UFFD_WP && !VM_MAYWRITE check, > it should have also added the 'ret = -EPERM;' statement. Right, if there's a "fixes" then it should be the uffd-wp patch. Though I really think it won't happen... Firstly because hugetlbfs is not yet supported for uffd-wp, so the two "if" won't collapse, so no way to trigger it imho. More importantly we've got one check ahead of it: /* * UFFDIO_COPY will fill file holes even without * PROT_WRITE. This check enforces that if this is a * MAP_SHARED, the process has write permission to the backing * file. If VM_MAYWRITE is set it also enforces that on a * MAP_SHARED vma: there is no F_WRITE_SEAL and no further * F_WRITE_SEAL can be taken until the vma is destroyed. */ ret = -EPERM; if (unlikely(!(cur->vm_flags & VM_MAYWRITE))) goto out_unlock; AFAICT it will fail there directly when write perm is missing. My wild guess is that the 1st version of 63b2d4174c4ad1f (2020) came earlier than 29ec90660d (2018), however not needed anymore after the 2020 patch. Hence it's probably overlooked by me when I rebased. Summary: IMHO no bug to fix, but we can directly drop the latter check? Thanks, -- Peter Xu