Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp2975409rwd; Wed, 14 Jun 2023 09:32:35 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6bfbNetnLaG5QZbsF+CcsUiOfIVpjTodkPy/RkQzsDcvHb3YUwdVu6iOBhvvDc02fYO6Fc X-Received: by 2002:a05:6402:1012:b0:518:72f1:8211 with SMTP id c18-20020a056402101200b0051872f18211mr4702725edu.28.1686760355646; Wed, 14 Jun 2023 09:32:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686760355; cv=none; d=google.com; s=arc-20160816; b=EmVEtbt1bLFI6ERnGpCrQBj4dnj5JlYMAnOP00/DvJk0giJeQx80YxuvNPc6Co+Y7a zxzvOBRPEOifaUNU4+DkOhc6Wb7HTsO5UWSRUHqZlwgON/VuwBJPhKvkgbsIoAo9vD+Z 1tTo66vAGt1DlmYOi4FURFcWYUIOKZLpUkV4rhft0hAXOchalFpvgoREVe2Jwsf/aW0i jH28xHNgnMfwPPckRRJBoOfwqry91ry29T5ka7Pi/PMQTLMZSe/WkKmCG2qIYr0U52RN SAJTxFMEL6HCXsifW3QY772EDaEvn7uZ1xWeUI6K7ODF6gUmE+5ryb6V/GHW4LBRSqxJ iUag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=FhKKsDrj5NxG0kmdQxhpTPAVWRUsLQsIGYXNIq74kdI=; b=Q/f+JHxzZGVYvqFnyABBO4LnMyQWPHIJwjeZ6zy3O8RSwgIqC3CL6Z8WTHh3gWVZ5h YYInhcQ8T8HgMBLN4a+2zAOmq62A6EgxchdbWtazqGries7+Nc46xhwzQqfVwW8Pa3ot uH8tMGvjICNXNf65vNFnZ02YeI2XDGMaxCfpS0tR9nlNQwNfxhEEkiuUOGpx7IbXftXV 7lAJ7kU4BM9IWNBliJuQkENkEJdg8sXmzNe6FoGe30rc4Wg4DwtQ3O8rZ3NmUPbr2XN1 xotrcL197/oQMsPf15dGNvYKKiVPxsmK4li54x7EZhMD6MPCx0x9da4ZUZ+9Bj81RkAE yBmw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bpuETZfF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y5-20020a50e605000000b005162d807f69si9866027edm.8.2023.06.14.09.32.01; Wed, 14 Jun 2023 09:32:35 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bpuETZfF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236280AbjFNPrr (ORCPT + 99 others); Wed, 14 Jun 2023 11:47:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43444 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235579AbjFNPrq (ORCPT ); Wed, 14 Jun 2023 11:47:46 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B7D622979 for ; Wed, 14 Jun 2023 08:46:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686757593; 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=FhKKsDrj5NxG0kmdQxhpTPAVWRUsLQsIGYXNIq74kdI=; b=bpuETZfF1sYMM0TYeZ87iqTeNPGHiPaDRl5TJqCWgPSHOvZTUFfdep1kS/JVm7iUSrgNt4 aWRJ0JB1l/M02xiWwQ+wY7iURbUukSOzKweZeX00TIcE2QGOlDIyACYw/JsGdfMXQEos9x HbemPqeCtYIS+ApkeVV5Mtskr3jtjRA= 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_256_GCM_SHA384) id us-mta-596-pCrczpm8NbWV4dLcngMPGw-1; Wed, 14 Jun 2023 11:46:32 -0400 X-MC-Unique: pCrczpm8NbWV4dLcngMPGw-1 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-3f8283a3a7aso15558081cf.1 for ; Wed, 14 Jun 2023 08:46:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686757592; x=1689349592; 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=FhKKsDrj5NxG0kmdQxhpTPAVWRUsLQsIGYXNIq74kdI=; b=fEpFeo1DDVoNoXaYZtvz4/qmodXkQfcHj7RaLGOBbrF0pS5VkUoW3rUSUpLJbPaD7s s+4ykgR7x6OonBCckV94UEC/GrV36Q0P5Eo+QCyLJD00EEIpbhhhmAVqo9vE/fngB7gP g7YGuelcXAe6UO4lPkMfeisMvk/TYZdMY2GoNldxhEzYbLcbvKsd/aXkwWL+RfBK+hz9 tQ6/F1ioZs7vMq66mbfeIEEhe2/kPGrEVyHWbbLbFtsq/N0BML+JOP9Ao09GUe8wXst3 zDVpva4sgCYnWlVJat8Z8I/8t5twCJTb9HqtSQrldkWdeR0mPCzoWoT1Hxu9q/sOELsC PSSA== X-Gm-Message-State: AC+VfDwi2biIlBQJM9t/oE2Gq+y9eqqhSi2UhabFaGYzEQ3La9897eqn zSCQn+btcoSGXiyRqcXDdtg8vgZtFlTD0xq6ENOc3tcFEMU+vreDyb4eqxb+G58RyQ3k2R0ErDo 8yqTPqC2SfbhxgJl0PcAfIzpo X-Received: by 2002:a05:622a:253:b0:3f9:aa64:7dab with SMTP id c19-20020a05622a025300b003f9aa647dabmr19576245qtx.6.1686757591897; Wed, 14 Jun 2023 08:46:31 -0700 (PDT) X-Received: by 2002:a05:622a:253:b0:3f9:aa64:7dab with SMTP id c19-20020a05622a025300b003f9aa647dabmr19576225qtx.6.1686757591551; Wed, 14 Jun 2023 08:46:31 -0700 (PDT) Received: from x1n (cpe5c7695f3aee0-cm5c7695f3aede.cpe.net.cable.rogers.com. [99.254.144.39]) by smtp.gmail.com with ESMTPSA id i6-20020ac84886000000b003f6bbd7863csm5091748qtq.86.2023.06.14.08.46.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Jun 2023 08:46:30 -0700 (PDT) Date: Wed, 14 Jun 2023 11:46:29 -0400 From: Peter Xu To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Matthew Wilcox , Andrea Arcangeli , John Hubbard , Mike Rapoport , Vlastimil Babka , "Kirill A . Shutemov" , Andrew Morton , Mike Kravetz , James Houghton , Hugh Dickins Subject: Re: [PATCH 2/7] mm/hugetlb: Fix hugetlb_follow_page_mask() on permission checks Message-ID: References: <20230613215346.1022773-1-peterx@redhat.com> <20230613215346.1022773-3-peterx@redhat.com> <24bc512a-b5c2-b7ea-fa83-5752cec7455b@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <24bc512a-b5c2-b7ea-fa83-5752cec7455b@redhat.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 14, 2023 at 05:31:36PM +0200, David Hildenbrand wrote: > On 13.06.23 23:53, Peter Xu wrote: > > It seems hugetlb_follow_page_mask() was missing permission checks. For > > example, one follow_page() can get the hugetlb page with FOLL_WRITE even if > > the page is read-only. > > I'm curious if there even is a follow_page() user that operates on hugetlb > ... > > s390x secure storage does not apply to hugetlb IIRC. You're the expert, so I'll rely on you. :) > > ksm.c? no. > > huge_memory.c ? no > > So what remains is most probably mm/migrate.c, which never sets FOLL_WRITE. > > Or am I missing something a user? Yes, non of the rest are with WRITE. Then I assume no fixes /backport needed at all (which is what this patch already does). It's purely to be prepared only. I'll mention that in the new version. Thanks, > > > > And it wasn't there even in the old follow_page_mask(), where we can > > reference from before commit 57a196a58421 ("hugetlb: simplify hugetlb > > handling in follow_page_mask"). > > > > Let's add them, namely, either the need to CoW due to missing write bit, or > > proper CoR on !AnonExclusive pages over R/O pins to reject the follow page. > > That brings this function closer to follow_hugetlb_page(). > > > > I just doubt how many of us care for that, for FOLL_PIN follow_page doesn't > > really happen at all. But we'll care, and care more if we switch over > > slow-gup to use hugetlb_follow_page_mask(). We'll also care when to return > > -EMLINK then, as that's the gup internal api to mean "we should do CoR". > > > > When at it, switching the try_grab_page() to use WARN_ON_ONCE(), to be > > clear that it just should never fail. > > > > Signed-off-by: Peter Xu > > --- > > mm/hugetlb.c | 22 ++++++++++++++++------ > > 1 file changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index 82dfdd96db4c..9c261921b2cf 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -6481,8 +6481,21 @@ struct page *hugetlb_follow_page_mask(struct vm_area_struct *vma, > > ptl = huge_pte_lock(h, mm, pte); > > entry = huge_ptep_get(pte); > > if (pte_present(entry)) { > > - page = pte_page(entry) + > > - ((address & ~huge_page_mask(h)) >> PAGE_SHIFT); > > + page = pte_page(entry); > > + > > + if (gup_must_unshare(vma, flags, page)) { > > + /* Tell the caller to do Copy-On-Read */ > > + page = ERR_PTR(-EMLINK); > > + goto out; > > + } > > + > > + if ((flags & FOLL_WRITE) && !pte_write(entry)) { > > + page = NULL; > > + goto out; > > + } > > + > > + page += ((address & ~huge_page_mask(h)) >> PAGE_SHIFT); > > + > > /* > > * Note that page may be a sub-page, and with vmemmap > > * optimizations the page struct may be read only. > > @@ -6492,10 +6505,7 @@ struct page *hugetlb_follow_page_mask(struct vm_area_struct *vma, > > * try_grab_page() should always be able to get the page here, > > * because we hold the ptl lock and have verified pte_present(). > > */ > > - if (try_grab_page(page, flags)) { > > - page = NULL; > > - goto out; > > - } > > + WARN_ON_ONCE(try_grab_page(page, flags)); > > } > > out: > > spin_unlock(ptl); > > -- > Cheers, > > David / dhildenb > -- Peter Xu