linux-kernel - Re: [PATCHv2] mm, page_vma_mapped: Drop faulty pointer arithmetics in check

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] [day] [month] [year] [list]

Message-ID: <CA+55aFxobYQ5cqnCZuf8xVWr3hCUmg=rTxDPV3zHWqeQysVkxA@mail.gmail.com>
Date:   Fri, 19 Jan 2018 10:01:54 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...nel.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-mm <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCHv2] mm, page_vma_mapped: Drop faulty pointer arithmetics in check_pte()

On Fri, Jan 19, 2018 at 4:49 AM, Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> wrote:
>
> +       if (pfn < page_to_pfn(pvmw->page))
> +               return false;
> +
> +       /* THP can be referenced by any subpage */
> +       if (pfn - page_to_pfn(pvmw->page) >= hpage_nr_pages(pvmw->page))
> +               return false;
> +

Is gcc actually clever enough to merge these? The "page_to_pfn()"
logic can be pretty expensive (exactly for the sparsemem case, but
per-node DISCOTIGMEM has some complexity too.

So I'd prefer to make that explicit, perhaps by having a helper
function that does this something like

   static inline bool pfn_in_hpage(unsigned long pfn, struct page *hpage)
   {
        unsigned long hpage_pfn = page_to_pfn(hpage);

        return pfn >= hpage_pfn &&  pfn - hpage_pfn < hpage_nr_pages(hpage);
    }

and then just use

    return pfn_in_hpage(pfn, pvmw->page);

in that caller. Hmm? Wouldn't that be more legible, and avoid the
repeated pvmw->page and page_to_pfn() cases?

Even if maybe gcc can do the CSE and turn it all into the same thing
in the end..

               Linus