* [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037) [not found] <52fdd350.dwn4aII31EyWlDq9%fengguang.wu@intel.com> @ 2014-02-14 13:04 ` Fengguang Wu 2014-02-14 15:09 ` [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() (Re: [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037)) Naoya Horiguchi [not found] ` <52fe31de.89cfe00a.338f.ffff9a19SMTPIN_ADDED_BROKEN@mx.google.com> 0 siblings, 2 replies; 4+ messages in thread From: Fengguang Wu @ 2014-02-14 13:04 UTC (permalink / raw) To: Naoya Horiguchi Cc: kbuild-all, Johannes Weiner, Andrew Morton, Linux Memory Management List Hi Naoya, FYI, there are new smatch warnings show up in tree: git://git.cmpxchg.org/linux-mmotm.git master head: 0363f94bc1c9b81f23ee7d2446331eb288568ea7 commit: 81272031cc2831a3d1abb3c681f1188aa36a1454 [97/220] pagewalk: remove argument hmask from hugetlb_entry() fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037) vim +/vma +1042 fs/proc/task_mmu.c d9104d1c Cyrill Gorcunov 2013-09-11 1031 int flags2; 16fbdce6 Konstantin Khlebnikov 2012-05-10 1032 pagemap_entry_t pme; 81272031 Naoya Horiguchi 2014-02-13 1033 unsigned long hmask; 5dc37642 Naoya Horiguchi 2009-12-14 1034 d9104d1c Cyrill Gorcunov 2013-09-11 1035 WARN_ON_ONCE(!vma); d9104d1c Cyrill Gorcunov 2013-09-11 1036 d9104d1c Cyrill Gorcunov 2013-09-11 @1037 if (vma && (vma->vm_flags & VM_SOFTDIRTY)) d9104d1c Cyrill Gorcunov 2013-09-11 1038 flags2 = __PM_SOFT_DIRTY; d9104d1c Cyrill Gorcunov 2013-09-11 1039 else d9104d1c Cyrill Gorcunov 2013-09-11 1040 flags2 = 0; d9104d1c Cyrill Gorcunov 2013-09-11 1041 21a2f342 Naoya Horiguchi 2014-02-13 @1042 hmask = huge_page_mask(hstate_vma(vma)); 5dc37642 Naoya Horiguchi 2009-12-14 1043 for (; addr != end; addr += PAGE_SIZE) { 116354d1 Naoya Horiguchi 2010-04-06 1044 int offset = (addr & ~hmask) >> PAGE_SHIFT; d9104d1c Cyrill Gorcunov 2013-09-11 1045 huge_pte_to_pagemap_entry(&pme, pm, *pte, offset, flags2); --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() (Re: [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037)) 2014-02-14 13:04 ` [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037) Fengguang Wu @ 2014-02-14 15:09 ` Naoya Horiguchi [not found] ` <52fe31de.89cfe00a.338f.ffff9a19SMTPIN_ADDED_BROKEN@mx.google.com> 1 sibling, 0 replies; 4+ messages in thread From: Naoya Horiguchi @ 2014-02-14 15:09 UTC (permalink / raw) To: Fengguang Wu Cc: kbuild-all, Johannes Weiner, Andrew Morton, Linux Memory Management List Hi Fengguang, On Fri, Feb 14, 2014 at 09:04:50PM +0800, Fengguang Wu wrote: ... > FYI, there are new smatch warnings show up in > > tree: git://git.cmpxchg.org/linux-mmotm.git master > head: 0363f94bc1c9b81f23ee7d2446331eb288568ea7 > commit: 81272031cc2831a3d1abb3c681f1188aa36a1454 [97/220] pagewalk: remove argument hmask from hugetlb_entry() > > fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037) > > vim +/vma +1042 fs/proc/task_mmu.c > > d9104d1c Cyrill Gorcunov 2013-09-11 1031 int flags2; > 16fbdce6 Konstantin Khlebnikov 2012-05-10 1032 pagemap_entry_t pme; > 81272031 Naoya Horiguchi 2014-02-13 1033 unsigned long hmask; > 5dc37642 Naoya Horiguchi 2009-12-14 1034 > d9104d1c Cyrill Gorcunov 2013-09-11 1035 WARN_ON_ONCE(!vma); > d9104d1c Cyrill Gorcunov 2013-09-11 1036 > d9104d1c Cyrill Gorcunov 2013-09-11 @1037 if (vma && (vma->vm_flags & VM_SOFTDIRTY)) > d9104d1c Cyrill Gorcunov 2013-09-11 1038 flags2 = __PM_SOFT_DIRTY; > d9104d1c Cyrill Gorcunov 2013-09-11 1039 else > d9104d1c Cyrill Gorcunov 2013-09-11 1040 flags2 = 0; > d9104d1c Cyrill Gorcunov 2013-09-11 1041 > 21a2f342 Naoya Horiguchi 2014-02-13 @1042 hmask = huge_page_mask(hstate_vma(vma)); > 5dc37642 Naoya Horiguchi 2009-12-14 1043 for (; addr != end; addr += PAGE_SIZE) { > 116354d1 Naoya Horiguchi 2010-04-06 1044 int offset = (addr & ~hmask) >> PAGE_SHIFT; > d9104d1c Cyrill Gorcunov 2013-09-11 1045 huge_pte_to_pagemap_entry(&pme, pm, *pte, offset, flags2); Thanks for reporting, here is a patch. We never have NULL vma in pagemap_hugetlb(), I added the BUG_ON check. Thanks, Naoya Horiguchi --- From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Date: Fri, 14 Feb 2014 09:35:06 -0500 Subject: [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() 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(). Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> --- fs/proc/task_mmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index f819d0d4a0e8..69aed7192254 100644 --- 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); - if (vma && (vma->vm_flags & VM_SOFTDIRTY)) + if (vma->vm_flags & VM_SOFTDIRTY) flags2 = __PM_SOFT_DIRTY; else flags2 = 0; -- 1.8.5.3 -- 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> ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <52fe31de.89cfe00a.338f.ffff9a19SMTPIN_ADDED_BROKEN@mx.google.com>]
* Re: [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() (Re: [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037)) [not found] ` <52fe31de.89cfe00a.338f.ffff9a19SMTPIN_ADDED_BROKEN@mx.google.com> @ 2014-02-18 20:44 ` Andrew Morton 2014-02-18 21:01 ` Naoya Horiguchi 0 siblings, 1 reply; 4+ messages in thread From: Andrew Morton @ 2014-02-18 20:44 UTC (permalink / raw) To: Naoya Horiguchi Cc: Fengguang Wu, kbuild-all, Johannes Weiner, Linux Memory Management List On Fri, 14 Feb 2014 10:09:58 -0500 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> 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. 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 -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() (Re: [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037)) 2014-02-18 20:44 ` Andrew Morton @ 2014-02-18 21:01 ` Naoya Horiguchi 0 siblings, 0 replies; 4+ messages in thread From: Naoya Horiguchi @ 2014-02-18 21:01 UTC (permalink / raw) To: akpm; +Cc: fengguang.wu, kbuild-all, hannes, linux-mm 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@ah.jp.nec.com> 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@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> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-18 21:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <52fdd350.dwn4aII31EyWlDq9%fengguang.wu@intel.com>
2014-02-14 13:04 ` [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037) Fengguang Wu
2014-02-14 15:09 ` [PATCH] fs/proc/task_mmu.c: assume non-NULL vma in pagemap_hugetlb() (Re: [mmotm:master 97/220] fs/proc/task_mmu.c:1042 pagemap_hugetlb() error: we previously assumed 'vma' could be null (see line 1037)) Naoya Horiguchi
[not found] ` <52fe31de.89cfe00a.338f.ffff9a19SMTPIN_ADDED_BROKEN@mx.google.com>
2014-02-18 20:44 ` Andrew Morton
2014-02-18 21:01 ` Naoya Horiguchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).