linux-kernel - Re: [PATCH v2 01/46] hugetlb: don't set PageUptodate for UFFDIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHS8izM5Es+DMiOyPt93o6Xw+SeuOR45r+MMoeagiYN594NiyQ@mail.gmail.com>
Date:   Fri, 17 Feb 2023 16:41:57 -0800
From:   Mina Almasry <almasrymina@...gle.com>
To:     James Houghton <jthoughton@...gle.com>
Cc:     Mike Kravetz <mike.kravetz@...cle.com>,
        Muchun Song <songmuchun@...edance.com>,
        Peter Xu <peterx@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        David Rientjes <rientjes@...gle.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        "Zach O'Keefe" <zokeefe@...gle.com>,
        Manish Mishra <manish.mishra@...anix.com>,
        Naoya Horiguchi <naoya.horiguchi@....com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Yang Shi <shy828301@...il.com>,
        Frank van der Linden <fvdl@...gle.com>,
        Jiaqi Yan <jiaqiyan@...gle.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 01/46] hugetlb: don't set PageUptodate for UFFDIO_CONTINUE

On Fri, Feb 17, 2023 at 4:28 PM James Houghton <jthoughton@...gle.com> wrote:
>
> If would be bad if we actually set PageUptodate with UFFDIO_CONTINUE;
> PageUptodate indicates that the page has been zeroed, and we don't want
> to give a non-zeroed page to the user.
>
> The reason this change is being made now is because UFFDIO_CONTINUEs on
> subpages definitely shouldn't set this page flag on the head page.
>
> Signed-off-by: James Houghton <jthoughton@...gle.com>
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 07abcb6eb203..792cb2e67ce5 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6256,7 +6256,16 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
>          * preceding stores to the page contents become visible before
>          * the set_pte_at() write.
>          */
> -       __folio_mark_uptodate(folio);
> +       if (!is_continue)
> +               __folio_mark_uptodate(folio);
> +       else if (!folio_test_uptodate(folio)) {
> +               /*
> +                * This should never happen; HugeTLB pages are always Uptodate
> +                * as soon as they are allocated.
> +                */

if (is_continue) then we grab a page from the page cache, no? Are
pages in page caches always uptodate? Why? I guess that means they're
mapped hence uptodate?

Also this comment should explain why pages in the page cache are
always uptodate, no? Because this error branch is hit if (is_continue
&& !folio_test_uptodate()), not when pages are freshly allocated.

> +               ret = -EFAULT;
> +               goto out_release_nounlock;
> +       }
>
>         /* Add shared, newly allocated pages to the page cache. */
>         if (vm_shared && !is_continue) {
> --
> 2.39.2.637.g21b0678d19-goog
>