Re: mm: kernel BUG at mm/mempolicy.c:1203!

[Sasha Levin <sasha.levin@oracle.com>]

On 12/17/2013 01:55 AM, Wanpeng Li wrote:
> Hi Sasha,
> On Tue, Dec 17, 2013 at 01:46:42AM -0500, Sasha Levin wrote:
>> On 12/17/2013 01:11 AM, Naoya Horiguchi wrote:
>>> Hello Bob,
>>>
>>> On Tue, Dec 17, 2013 at 12:38:49PM +0800, Bob Liu wrote:
>>>> On 12/17/2013 09:10 AM, Sasha Levin wrote:
>>>>> On 12/16/2013 07:44 PM, Bob Liu wrote:
>>>>>>
>>>>>> On 12/16/2013 07:37 AM, Sasha Levin wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> While fuzzing with trinity inside a KVM tools guest running latest -next
>>>>>>> kernel, I've
>>>>>>> stumbled on the following spew.
>>>>>>>
>>>>>>> This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in
>>>>>>> new_vma_page()"
>>>>>>> which added that BUG_ON.
>>>>>>
>>>>>> Could you take a try with this patch from Wanpeng Li?
>>>>>>
>>>>>> Thanks,
>>>>>> -Bob
>>>>>>
>>>>>> Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
>>>>>> ....
>>>>>> Signed-off-by: Wanpeng Li <liwanp@linux.vnet.ibm.com>
>>>>>> index eca4a31..73b5a35 100644
>>>>>> --- a/mm/mempolicy.c
>>>>>> +++ b/mm/mempolicy.c
>>>>>> @@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
>>>>>> *page, unsigned long private, int *
>>>>>>                 break;
>>>>>>             vma = vma->vm_next;
>>>>>>         }
>>>>>> +
>>>>>> +    if (PageHuge(page)) {
>>>>>> +        if (vma)
>>>>>> +            return alloc_huge_page_noerr(vma, address, 1);
>>>>>> +        else
>>>>>> +            return NULL;
>>>>>> +    }
>>>>>>         /*
>>>>>> -     * queue_pages_range() confirms that @page belongs to some vma,
>>>>>> -     * so vma shouldn't be NULL.
>>>>>> +     * if !vma, alloc_page_vma() will use task or system default policy
>>>>>>          */
>>>>>> -    BUG_ON(!vma);
>>>>>> -
>>>>>> -    if (PageHuge(page))
>>>>>> -        return alloc_huge_page_noerr(vma, address, 1);
>>>>>>         return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
>>>>>>     }
>>>>>>     #else
>>>>>>
>>>>>
>>>>> Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed
>>>>> pretty certain that this
>>>>> situation shouldn't happen at all. What's the reasoning behind just
>>>>
>>>> I think this assumption may not correct.
>>>> Even if
>>>> address = __vma_address(page, vma);
>>>> and
>>>> vma->start < address < vma->end;
>>>> page_address_in_vma() may still return -EFAULT because of many other
>>>> conditions in it.
>>>> As a result the while loop in new_vma_page() may end with vma=NULL.
>>>>
>>>> Naoya, any idea?
>>>
>>> Yes, you totally make sense. So please apply Wanpeng's patch.
>>
>> Shouldn't it just be a revert of Naoya's patch? Otherwise we're
>> changing code paths unnecessarily.
>>
>
> Actually, the original target of Naoya's patch is try to fix potential dereference
> NULL pointer by Dan. http://marc.info/?l=linux-mm&m=137689530323257&w=2
>
> This patch fix both the regression and potential dereference NULL pointer reported
> by Dan. http://marc.info/?l=linux-kernel&m=138726268626705&w=2

Makes sense, thanks!


Thanks,
Sasha

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>