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  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]
Date:   Sat, 26 Dec 2020 13:07:14 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     Hugh Dickins <hughd@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Will Deacon <will@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Jan Kara <jack@...e.cz>, Minchan Kim <minchan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vinayak Menon <vinmenon@...eaurora.org>,
        Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting

I was going to just apply this patch, because I like it so much, but
then I decided to take one last look, and:

On Sat, Dec 26, 2020 at 12:43 PM Kirill A. Shutemov
<kirill@...temov.name> wrote:
>
> +static bool filemap_map_pmd(struct vm_fault *vmf, struct page *page)
> +{
> +       struct mm_struct *mm = vmf->vma->vm_mm;
> +
> +       /* Huge page is mapped? No need to proceed. */
> +       if (pmd_trans_huge(*vmf->pmd))
> +               return true;

doesn't this cause us to leak a locked page?

I get the feeling that every single "return true" case here should
always unlock the page and - with the exception of a successful
do_set_pmd() - do a "put_page()".

Which kind of argues that we should just do it in the caller (and get
an extra ref in the do_set_pmd() case, so that the caller can always
do

        if (filemap_map_pmd(..)) {
                unlock_page(page);
                put_page(page);
                rcu_read_unlock();
                return;
        }

andf then there are no odd cases inside that filemap_map_pmd() function. Hmm?

Other than that, I really find it all much more legible.

Of course, if I'm wrong about the above, that just proves that I'm
missing something and it wasn't so legible after all..

              Linus

Powered by blists - more mailing lists