linux-kernel - Re: could you clarify mm/mempolicy: fix !vma in new_vma

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: <CAA_GA1dh3TtzGnK0HgAb_Sy6ww5JBaFqmf_YViPKpMCEpzFh4w@mail.gmail.com>
Date:	Wed, 8 Jan 2014 21:10:29 +0800
From:	Bob Liu <lliubbo@...il.com>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	Sasha Levin <sasha.levin@...cle.com>,
	Wanpeng Li <liwanp@...ux.vnet.ibm.com>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Bob Liu <bob.liu@...cle.com>, Linux-MM <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: could you clarify mm/mempolicy: fix !vma in new_vma_page()

On Wed, Jan 8, 2014 at 8:42 PM, Michal Hocko <mhocko@...e.cz> wrote:
> On Wed 08-01-14 20:09:30, Bob Liu wrote:
>> On Wed, Jan 8, 2014 at 6:08 PM, Michal Hocko <mhocko@...e.cz> wrote:
>>
>> >
>> > If I was debugging this I would simply add printk into page_address_in_vma
>> > error paths.
>> >
>> > Anyway, I think that at least hugetlbfs part should be reverted because
>> > it might paper over real bugs. Although the migration would fail for
>> > such hugetlb page we should catch that a weird page was tried to be
>> > migrated. What about the patch below?
>>
>> Looks good to me. But we need to confirm whether our assumption is right.
>
> Which assumption you have in mind? non-linear mapping or failing on
> anon_vma or f_mapping checks?
>

The assumption that the original BUG_ON(!vma) was triggered by
non-linear mapping.

>> Sasha, could you please have a test with Michal's patch?
>
> I obviously doesn't have anything against testing but we should really
> focus on the original issue. This patch simply restores hugetlb code

Oh, I see your point.
Yes, I agree that your patch should be merged and if the BUG_ON() is
triggered(which is unlikely) again.
We can open another thread and analysis the root cause.

>> > ---
>> > From 2d61421f26a3b63b4670d71b7adc67e2191b6157 Mon Sep 17 00:00:00 2001
>> > From: Michal Hocko <mhocko@...e.cz>
>> > Date: Wed, 8 Jan 2014 10:57:41 +0100
>> > Subject: [PATCH] mm: new_vma_page cannot see NULL vma for hugetlb pages
>> >
>> > 11c731e81bb0 (mm/mempolicy: fix !vma in new_vma_page()) has removed
>> > BUG_ON(!vma) from new_vma_page which is partially correct because
>> > page_address_in_vma will return EFAULT for non-linear mappings and at
>> > least shared shmem might be mapped this way.
>> >
>> > The patch also tried to prevent NULL ptr for hugetlb pages which is not
>> > correct AFAICS because hugetlb pages cannot be mapped as VM_NONLINEAR
>> > and other conditions in page_address_in_vma seem to be legit and catch
>> > real bugs.
>> >
>> > This patch restores BUG_ON for PageHuge to catch potential issues when
>> > the to-be-migrated page is not setup properly.
>> >
>> > Signed-off-by: Michal Hocko <mhocko@...e.cz>

Reviewed-by: Bob Liu <bob.liu@...cle.com>

>> > ---
>> >  mm/mempolicy.c | 6 ++----
>> >  1 file changed, 2 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> > index 9e8d2d86978a..f3f51464a23b 100644
>> > --- a/mm/mempolicy.c
>> > +++ b/mm/mempolicy.c
>> > @@ -1199,10 +1199,8 @@ static struct page *new_vma_page(struct page *page, unsigned long private, int *
>> >         }
>> >
>> >         if (PageHuge(page)) {
>> > -               if (vma)
>> > -                       return alloc_huge_page_noerr(vma, address, 1);
>> > -               else
>> > -                       return NULL;
>> > +               BUG_ON(vma)
>> > +               return alloc_huge_page_noerr(vma, address, 1);
>> >         }
>> >         /*
>> >          * if !vma, alloc_page_vma() will use task or system default policy
>> > --
>> > 1.8.5.2
>> >

-- 
Regards,
--Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/