Re: [PATCH] mm: fix zero page check in vm_normal_page

[Konstantin Khlebnikov <koct9i@gmail.com>]

On Tue, May 27, 2014 at 4:10 PM, vinayak menon <vinayakm.list@gmail.com> wrote:
> On Tue, May 27, 2014 at 5:18 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>> On Tue, May 27, 2014 at 3:43 PM, vinayak menon <vinayakm.list@gmail.com> wrote:
>>> On Tue, May 27, 2014 at 5:01 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>>>> On Tue, May 27, 2014 at 2:31 PM, Kirill A. Shutemov
>>>> <kirill.shutemov@linux.intel.com> wrote:
>>>>> Vinayak Menon wrote:
>>>>>> An issue was observed when a userspace task exits.
>>>>>> The page which hits error here is the zero page.
>>>>>> In zap_pte_range, vm_normal_page gets called, and it
>>>>>> returns a page address and not NULL, even though the
>>>>>> pte corresponds to zero pfn. In this case,
>>>>>> HAVE_PTE_SPECIAL is not set, and VM_MIXEDMAP is set
>>>>>> in vm_flags. In the case of VM_MIXEDMAP , only pfn_valid
>>>>>> is checked, and not is_zero_pfn. This results in
>>>>>> zero page being returned instead of NULL.
>>>>>>
>>>>>> BUG: Bad page map in process mediaserver  pte:9dff379f pmd:9bfbd831
>>>>>> page:c0ed8e60 count:1 mapcount:-1 mapping:  (null) index:0x0
>>>>>> page flags: 0x404(referenced|reserved)
>>>>>> addr:40c3f000 vm_flags:10220051 anon_vma:  (null) mapping:d9fe0764 index:fd
>>>>>> vma->vm_ops->fault:   (null)
>>>>>> vma->vm_file->f_op->mmap: binder_mmap+0x0/0x274
>>>>>
>>>>> How do we get zero_pfn there. We shouldn't use zero page for file mappings.
>>>>> binder does some tricks?
>>>>
>>>> Its vm_ops doesn't provide ->fault method at all.
>>>> Seems like all ptes must be populated at the mmap time.
>>>> For some reason read page fault had happened and handle_pte_fault()
>>>> handled it in do_anonymous_page() which maps zero_page.
>>>>
>>> When the task crashed, it was ptraced by debuggered and the areas were
>>> dumped. And this resulted in the read page fault.
>>
>> Anyway, this bug in the binder. It must either populate all PTEs in ->mmap()
>> or provide ->fault() method. Falling into do_anonymous_page() isn't funny.
>
> Ok. But in vm_normal_page shouldn't we check for zero_pfn in the case
> of VM_MIXEDMAP ?

I don't think so. VM_MIXEDMAP shouldn't have zero_pfn.
It isn't expected to be here, for example in your case next fault for
write will remap
that zero page for write becaue do_wp_page() doesn't expect to see it here.

In case of HAVE_PTE_SPECIAL it's checked because zero_page _also_
installed as special pte and
this must be the only one possible special-pte in non-mixedmap/pfnmap vmas.

--
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>