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 9E1E3EB64D8 for ; Wed, 14 Jun 2023 15:46:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0AAE06B0074; Wed, 14 Jun 2023 11:46:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 05B316B0075; Wed, 14 Jun 2023 11:46:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E3E008E0001; Wed, 14 Jun 2023 11:46:36 -0400 (EDT) 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 D0C996B0074 for ; Wed, 14 Jun 2023 11:46:36 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 924801206DC for ; Wed, 14 Jun 2023 15:46:36 +0000 (UTC) X-FDA: 80901780792.20.99661B6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf05.hostedemail.com (Postfix) with ESMTP id 60328100012 for ; Wed, 14 Jun 2023 15:46:34 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=bpuETZfF; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf05.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686757594; 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=FhKKsDrj5NxG0kmdQxhpTPAVWRUsLQsIGYXNIq74kdI=; b=nJGNBUn3peKQGZf3Kji1HWnYShGAOFX/J8xAtl1WTd2tlsGf9bpXrSoaaRXxWZCwadPonn dvB38JVGQo98D3h2MtD1YqGLPALI/mVi3E0c73nrG2Ev6arRZcThnoJ5fgIiqqUGK0xo7a mXK4u7XzcEzm1RRdYJ/+ZI1RvitT54A= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=bpuETZfF; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf05.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686757594; a=rsa-sha256; cv=none; b=aEmuv2DRoHw5O0U/WcQ4XRk31ONMikYpoEr8BdjDGNqGFeFO5LyO4OIZmXjXEfJuYsmD8W jYw4l77VHN5kE3XyiMksWANt9z644hUpkNBd5eG4p73rBEvCkCOPEQWTQmJ28uuUNoEOYw iJ4p3Ejo9//qVCdPgZAnuylTRUtMMVI= 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-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-21-EAMmWSg6OWeQs8YnnICR9Q-1; Wed, 14 Jun 2023 11:46:32 -0400 X-MC-Unique: EAMmWSg6OWeQs8YnnICR9Q-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-3f8283a3a7aso15558101cf.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=Aw/oH7ZUd4Jf9KcaTDjq7DWB4cG3hhEB+HyDTebbAcwRrXOJt3vy541r1mcweDJH90 N3nqFENp01IBwunSFs1WOy7/0SP5fhvW5oPd6e2KZrB7hnXok1USV8LITIp/QaBsPATH 0u+La5kSNMB0Dy0fO20b1v9DbA10W5WbpM6AQoVDXo2lTElkFZ+CCaxXuigYjf6GJ9xE maAyXVvikN2E0jSf0BkqNWCfOUJWvgX4d0ga5J6hiaM5cjQq9A4cSNrQmE3o4t8dcISQ s7L5DjvcjJhvjN9TA0qtwYJZYv3Th+HoM1gMTvUpB2e49djQcdLhVrx8RSBpniMaYrcw uEEQ== X-Gm-Message-State: AC+VfDyG1neaMauXqQmnYyY8Wy7yQRLRgy+Bd0GKJYtdgFlMiRywMJFp PlO64i8zEY56X7BUuxtKRorKrLtjyUr5vek+15Svo+VTQ20Xgq3eTxIXDLprw5FelhyXtNv71BY KpfwMeMz4spQ= X-Received: by 2002:a05:622a:253:b0:3f9:aa64:7dab with SMTP id c19-20020a05622a025300b003f9aa647dabmr19576247qtx.6.1686757591896; Wed, 14 Jun 2023 08:46:31 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7NwlNl6Akhvm2MhXCy1HxMJsM1KyCbNtghJVerchCD0v7n3q2L6EQJ6+NPG0PruSvzN2hRFA== 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 In-Reply-To: <24bc512a-b5c2-b7ea-fa83-5752cec7455b@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Queue-Id: 60328100012 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: zohxpxcu3hxb8x7193wqre9uj978ic7s X-HE-Tag: 1686757594-812574 X-HE-Meta: U2FsdGVkX1+vOHhDgzDsTSSWumTLcypSQOa61TKo/qgaETdYCvhOCXg6O3M4SbmosbIgTY7wYwum1g2gHtV+GdlCBqwbEszSqNtFZXalFVnPSjoYBzmfUy2FUAagxU3tPV9ONkooePraWJXryKbz3lLeKoM+zo67g7VO1+vKQVxSnKxFE/4YiNY/7weYjRH0C2L8z2YjuERY5E614viWXuas4yrOnNF6FE32Z7436rD0LOhTDQ587eKYijgwTkhNHOcyY0NMVL3yLPzcIzrq0y4lPYGoqJJjfu05ySB+qkvBZkXdu1b457PIGaOxyRPaj2z4Efa1RxBK3QhCKnWqpE+IVP+hElHajj7RzlwSYtLgz06KBjht/oyqy9ANbKb2KH9zE4cdP3q+Tzj/xKvJtC8qOjDN07LJQw7WtqJJAo2n5Uso3N1HQDHx2Ug+1dflbNAhQIcONza18P9ZG4aJ2U6MA2C+T8nFRKU1PgJT/UlrVOOtpEiqLuieU2q5IFPnnRClwrWAeXqhnb6uXezPS4rBM0uAXbGf+0jt9t4XTHaGKuhZ7F3CNjvy93Td4SYfZLFuieRRKuHoWvx0iwzK1JcCYwlexIXUmOoY9DqIcpbaA7PoWRF3ZKKuqQBvQV1BwMZCRGz2OkhzZ/eCvztSRVybzmZQmo6e5eqqNFO6kukY/fxQlxRy/jyrFWYocVxk9pXgiQh+4CF26u4bhPTq2R6dNj+i90THon8KP7JMZBOc1d5qdStjgDyN83ZAqDB9S/Zq1fBEqBfYHYRP19WmkH9JoMDKKKFrlYAjBW2WWP14iPNZWr/V7AVe5Kk4sruTPe2YOgyl3HBnuGQaJgkO5a2ashV8dCY9fD+AiGnPeiX5bNlvrYFrsceIOw7pl2MsavRBAMhtNHS34zQI+9sZRVTbNvzsCbjnK40UO73F3Yw2ThN9NAiVYoFzRv+u1Uynx4Iow55FF8brkiGHfhQ Q3mN2bg2 8ybFo3XEh8qh8iqNDaEDC6SEYNaqPZUGdLe2Wws/4P60FD6clBfDu70+kBbkrbuzlpRo1+ZD6JdoLO8L1aHLoost96RdUlUvvLMZqjMAMc8X0smTk5NrNIXgo31FhIYCDXJTYhMGEt0qCR/ttf4E1drcT0/80vqPxlOfmwWNbg5dEThRzPruXz2PWdraTk9TtXw8XkQdlfZkr9RHSpSyFgUy/Mi7R72lKdwA9DjQcd/11aEvFpCZpvnGBWZl6qtKoptqGb3nVxqhxkmLac3WxyUuld4hLpn3wfX4MQ0m3yaueVpFXnJjaibHlCjx8Fl+6aOU47G/rGntEd9EbfZWo3JxN/Dm6jPtX9FRv2Tf7VrLwaKY1pPJi6qpKh2Ud4rKrW9DOx5ziSmrqEa4TbfhbgzALK5W/sEgksmimUqwhq+Qx3D7PXVO9Qi9OIg== 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 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