On Tue, Feb 18, 2014 at 12:44:00PM -0800, Andrew Morton wrote: > On Fri, 14 Feb 2014 10:09:58 -0500 Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> wrote: > > > Fengguang reported smatch error about potential NULL pointer access. > > > > In updated page table walker, we never run ->hugetlb_entry() callback > > on the address without vma. This is because __walk_page_range() checks > > it in advance. So we can assume non-NULL vma in pagemap_hugetlb(). > > > > ... > > > > --- a/fs/proc/task_mmu.c > > +++ b/fs/proc/task_mmu.c > > @@ -1032,9 +1032,9 @@ static int pagemap_hugetlb(pte_t *pte, unsigned long addr, unsigned long end, > > pagemap_entry_t pme; > > unsigned long hmask; > > > > - WARN_ON_ONCE(!vma); > > + BUG_ON(!vma); > > Let's just remove it altogether. > > > - if (vma && (vma->vm_flags & VM_SOFTDIRTY)) > > + if (vma->vm_flags & VM_SOFTDIRTY) > > The null deref oops will provide us the same info as the BUG_ON. It > will require a *little* more thinking to work out that `vma' was NULL, > but it will be pretty obvious. Yes, I agree. pagemap_hugetlb() is callback and never inlined, so we will have an info like 'NULL pointer access on pagemap_hugetlb()' with call trace, which is enough to pinpoint the problem. > > This requires knowing offsetof(vm_area_struct, vm_flags). I use a gdb macro: > > define offsetof > set $off = &(((struct $arg0 *)0)->$arg1) > printf "%d 0x%x\n", $off, $off > end > > akpm3:/usr/src/25> gdb fs/proc/task_mmu.o > ... > (gdb) offsetof vm_area_struct vm_flags > 80 0x50 Nice note, thank you. Naoya -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>