* FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree @ 2017-11-22 8:37 gregkh 2017-11-22 9:47 ` Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko 0 siblings, 2 replies; 18+ messages in thread From: gregkh @ 2017-11-22 8:37 UTC (permalink / raw) To: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, vbabka Cc: stable The patch below does not apply to the 4.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable@vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ >From e492080e640c2d1235ddf3441cae634cfffef7e1 Mon Sep 17 00:00:00 2001 From: Jaewon Kim <jaewon31.kim@samsung.com> Date: Wed, 15 Nov 2017 17:39:07 -0800 Subject: [PATCH] mm/page_ext.c: check if page_ext is not prepared online_page_ext() and page_ext_init() allocate page_ext for each section, but they do not allocate if the first PFN is !pfn_present(pfn) or !pfn_valid(pfn). Then section->page_ext remains as NULL. lookup_page_ext checks NULL only if CONFIG_DEBUG_VM is enabled. For a valid PFN, __set_page_owner will try to get page_ext through lookup_page_ext. Without CONFIG_DEBUG_VM lookup_page_ext will misuse NULL pointer as value 0. This incurrs invalid address access. This is the panic example when PFN 0x100000 is not valid but PFN 0x13FC00 is being used for page_ext. section->page_ext is NULL, get_entry returned invalid page_ext address as 0x1DFA000 for a PFN 0x13FC00. To avoid this panic, CONFIG_DEBUG_VM should be removed so that page_ext will be checked at all times. Unable to handle kernel paging request at virtual address 01dfa014 ------------[ cut here ]------------ Kernel BUG at ffffff80082371e0 [verbose debug info unavailable] Internal error: Oops: 96000045 [#1] PREEMPT SMP Modules linked in: PC is at __set_page_owner+0x48/0x78 LR is at __set_page_owner+0x44/0x78 __set_page_owner+0x48/0x78 get_page_from_freelist+0x880/0x8e8 __alloc_pages_nodemask+0x14c/0xc48 __do_page_cache_readahead+0xdc/0x264 filemap_fault+0x2ac/0x550 ext4_filemap_fault+0x3c/0x58 __do_fault+0x80/0x120 handle_mm_fault+0x704/0xbb0 do_page_fault+0x2e8/0x394 do_mem_abort+0x88/0x124 Pre-4.7 kernels also need commit f86e4271978b ("mm: check the return value of lookup_page_ext for all call sites"). Link: http://lkml.kernel.org/r/20171107094131.14621-1-jaewon31.kim@samsung.com Fixes: eefa864b701d ("mm/page_ext: resurrect struct page extending code for debugging") Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com> Acked-by: Michal Hocko <mhocko@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <js1304@gmail.com> Cc: <stable@vger.kernel.org> [depends on f86e427197, see above] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> diff --git a/mm/page_ext.c b/mm/page_ext.c index 4f0367d472c4..2c16216c29b6 100644 --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -125,7 +125,6 @@ struct page_ext *lookup_page_ext(struct page *page) struct page_ext *base; base = NODE_DATA(page_to_nid(page))->node_page_ext; -#if defined(CONFIG_DEBUG_VM) /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -134,7 +133,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (unlikely(!base)) return NULL; -#endif index = pfn - round_down(node_start_pfn(page_to_nid(page)), MAX_ORDER_NR_PAGES); return get_entry(base, index); @@ -199,7 +197,6 @@ struct page_ext *lookup_page_ext(struct page *page) { unsigned long pfn = page_to_pfn(page); struct mem_section *section = __pfn_to_section(pfn); -#if defined(CONFIG_DEBUG_VM) /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -208,7 +205,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (!section->page_ext) return NULL; -#endif return get_entry(section->page_ext, pfn); } ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree 2017-11-22 8:37 FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree gregkh @ 2017-11-22 9:47 ` Michal Hocko 2017-11-22 9:54 ` Greg KH 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko 1 sibling, 1 reply; 18+ messages in thread From: Michal Hocko @ 2017-11-22 9:47 UTC (permalink / raw) To: gregkh; +Cc: jaewon31.kim, akpm, js1304, minchan, stable, torvalds, vbabka Greg, the patch is clear about its dependecy for pre-4.7 kernels. I do not see f86e4271978b queued for the stable tree though. Is it just me not seeing it or your automation doesn't check for such dependencies? On Wed 22-11-17 09:37:57, Greg KH wrote: > >From e492080e640c2d1235ddf3441cae634cfffef7e1 Mon Sep 17 00:00:00 2001 > From: Jaewon Kim <jaewon31.kim@samsung.com> > Date: Wed, 15 Nov 2017 17:39:07 -0800 > Subject: [PATCH] mm/page_ext.c: check if page_ext is not prepared > > online_page_ext() and page_ext_init() allocate page_ext for each > section, but they do not allocate if the first PFN is !pfn_present(pfn) > or !pfn_valid(pfn). Then section->page_ext remains as NULL. > lookup_page_ext checks NULL only if CONFIG_DEBUG_VM is enabled. For a > valid PFN, __set_page_owner will try to get page_ext through > lookup_page_ext. Without CONFIG_DEBUG_VM lookup_page_ext will misuse > NULL pointer as value 0. This incurrs invalid address access. > > This is the panic example when PFN 0x100000 is not valid but PFN > 0x13FC00 is being used for page_ext. section->page_ext is NULL, > get_entry returned invalid page_ext address as 0x1DFA000 for a PFN > 0x13FC00. > > To avoid this panic, CONFIG_DEBUG_VM should be removed so that page_ext > will be checked at all times. > > Unable to handle kernel paging request at virtual address 01dfa014 > ------------[ cut here ]------------ > Kernel BUG at ffffff80082371e0 [verbose debug info unavailable] > Internal error: Oops: 96000045 [#1] PREEMPT SMP > Modules linked in: > PC is at __set_page_owner+0x48/0x78 > LR is at __set_page_owner+0x44/0x78 > __set_page_owner+0x48/0x78 > get_page_from_freelist+0x880/0x8e8 > __alloc_pages_nodemask+0x14c/0xc48 > __do_page_cache_readahead+0xdc/0x264 > filemap_fault+0x2ac/0x550 > ext4_filemap_fault+0x3c/0x58 > __do_fault+0x80/0x120 > handle_mm_fault+0x704/0xbb0 > do_page_fault+0x2e8/0x394 > do_mem_abort+0x88/0x124 > > Pre-4.7 kernels also need commit f86e4271978b ("mm: check the return > value of lookup_page_ext for all call sites"). > > Link: http://lkml.kernel.org/r/20171107094131.14621-1-jaewon31.kim@samsung.com > Fixes: eefa864b701d ("mm/page_ext: resurrect struct page extending code for debugging") > Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com> > Acked-by: Michal Hocko <mhocko@suse.com> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Joonsoo Kim <js1304@gmail.com> > Cc: <stable@vger.kernel.org> [depends on f86e427197, see above] > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > > diff --git a/mm/page_ext.c b/mm/page_ext.c > index 4f0367d472c4..2c16216c29b6 100644 > --- a/mm/page_ext.c > +++ b/mm/page_ext.c > @@ -125,7 +125,6 @@ struct page_ext *lookup_page_ext(struct page *page) > struct page_ext *base; > > base = NODE_DATA(page_to_nid(page))->node_page_ext; > -#if defined(CONFIG_DEBUG_VM) > /* > * The sanity checks the page allocator does upon freeing a > * page can reach here before the page_ext arrays are > @@ -134,7 +133,6 @@ struct page_ext *lookup_page_ext(struct page *page) > */ > if (unlikely(!base)) > return NULL; > -#endif > index = pfn - round_down(node_start_pfn(page_to_nid(page)), > MAX_ORDER_NR_PAGES); > return get_entry(base, index); > @@ -199,7 +197,6 @@ struct page_ext *lookup_page_ext(struct page *page) > { > unsigned long pfn = page_to_pfn(page); > struct mem_section *section = __pfn_to_section(pfn); > -#if defined(CONFIG_DEBUG_VM) > /* > * The sanity checks the page allocator does upon freeing a > * page can reach here before the page_ext arrays are > @@ -208,7 +205,6 @@ struct page_ext *lookup_page_ext(struct page *page) > */ > if (!section->page_ext) > return NULL; > -#endif > return get_entry(section->page_ext, pfn); > } > > -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree 2017-11-22 9:47 ` Michal Hocko @ 2017-11-22 9:54 ` Greg KH 2017-11-22 12:08 ` Michal Hocko 0 siblings, 1 reply; 18+ messages in thread From: Greg KH @ 2017-11-22 9:54 UTC (permalink / raw) To: Michal Hocko Cc: jaewon31.kim, akpm, js1304, minchan, stable, torvalds, vbabka On Wed, Nov 22, 2017 at 10:47:57AM +0100, Michal Hocko wrote: > Greg, > the patch is clear about its dependecy for pre-4.7 kernels. I do not see > f86e4271978b queued for the stable tree though. Is it just me not seeing > it or your automation doesn't check for such dependencies? Yes, I tried to apply the dependency, but that too failed, so I gave a "FAILED" email response for that patch, as well as this one so that people can help me out :) thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree 2017-11-22 9:54 ` Greg KH @ 2017-11-22 12:08 ` Michal Hocko 0 siblings, 0 replies; 18+ messages in thread From: Michal Hocko @ 2017-11-22 12:08 UTC (permalink / raw) To: Greg KH; +Cc: jaewon31.kim, akpm, js1304, minchan, stable, torvalds, vbabka On Wed 22-11-17 10:54:49, Greg KH wrote: > On Wed, Nov 22, 2017 at 10:47:57AM +0100, Michal Hocko wrote: > > Greg, > > the patch is clear about its dependecy for pre-4.7 kernels. I do not see > > f86e4271978b queued for the stable tree though. Is it just me not seeing > > it or your automation doesn't check for such dependencies? > > Yes, I tried to apply the dependency, but that too failed, so I gave a > "FAILED" email response for that patch, as well as this one so that > people can help me out :) Ahh, OK, I didn't see that one as it didn't land neither in lkml nor in my inbox. Anyway, I will send both patches as a reply to your original email. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-22 8:37 FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree gregkh 2017-11-22 9:47 ` Michal Hocko @ 2017-11-22 12:09 ` Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 2/2] mm/page_ext.c: check if page_ext is not prepared Michal Hocko ` (3 more replies) 1 sibling, 4 replies; 18+ messages in thread From: Michal Hocko @ 2017-11-22 12:09 UTC (permalink / raw) To: Greg KH Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim From: Yang Shi <yang.shi@linaro.org> commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. Per the discussion with Joonsoo Kim [1], we need check the return value of lookup_page_ext() for all call sites since it might return NULL in some cases, although it is unlikely, i.e. memory hotplug. Tested with ltp with "page_owner=0". [1] http://lkml.kernel.org/r/20160519002809.GA10245@js1304-P5Q-DELUXE [akpm@linux-foundation.org: fix build-breaking typos] [arnd@arndb.de: fix build problems from lookup_page_ext] Link: http://lkml.kernel.org/r/6285269.2CksypHdYp@wuerfel [akpm@linux-foundation.org: coding-style fixes] Link: http://lkml.kernel.org/r/1464023768-31025-1-git-send-email-yang.shi@linaro.org Signed-off-by: Yang Shi <yang.shi@linaro.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Michal Hocko <mhocko@suse.com> --- include/linux/page_idle.h | 43 ++++++++++++++++++++++++++++++++++++------- mm/debug-pagealloc.c | 6 ++++++ mm/page_alloc.c | 6 ++++++ mm/page_owner.c | 16 ++++++++++++++++ mm/vmstat.c | 2 ++ 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/include/linux/page_idle.h b/include/linux/page_idle.h index bf268fa92c5b..fec40271339f 100644 --- a/include/linux/page_idle.h +++ b/include/linux/page_idle.h @@ -46,33 +46,62 @@ extern struct page_ext_operations page_idle_ops; static inline bool page_is_young(struct page *page) { - return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline void set_page_young(struct page *page) { - set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool test_and_clear_page_young(struct page *page) { - return test_and_clear_bit(PAGE_EXT_YOUNG, - &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_and_clear_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool page_is_idle(struct page *page) { - return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void set_page_idle(struct page *page) { - set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void clear_page_idle(struct page *page) { - clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + clear_bit(PAGE_EXT_IDLE, &page_ext->flags); } #endif /* CONFIG_64BIT */ diff --git a/mm/debug-pagealloc.c b/mm/debug-pagealloc.c index 5bf5906ce13b..fe1c61f7cf26 100644 --- a/mm/debug-pagealloc.c +++ b/mm/debug-pagealloc.c @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return; __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return; __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (page_ext) + return false; return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 6b5421ae86c6..38aca81deeaf 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -560,6 +560,9 @@ static inline void set_page_guard(struct zone *zone, struct page *page, return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __set_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); INIT_LIST_HEAD(&page->lru); @@ -577,6 +580,9 @@ static inline void clear_page_guard(struct zone *zone, struct page *page, return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __clear_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); set_page_private(page, 0); diff --git a/mm/page_owner.c b/mm/page_owner.c index 983c3a10fa07..dd6b9cebf981 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -53,6 +53,8 @@ void __reset_page_owner(struct page *page, unsigned int order) for (i = 0; i < (1 << order); i++) { page_ext = lookup_page_ext(page + i); + if (unlikely(!page_ext)) + continue; __clear_bit(PAGE_EXT_OWNER, &page_ext->flags); } } @@ -60,6 +62,7 @@ void __reset_page_owner(struct page *page, unsigned int order) void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) { struct page_ext *page_ext = lookup_page_ext(page); + struct stack_trace trace = { .nr_entries = 0, .max_entries = ARRAY_SIZE(page_ext->trace_entries), @@ -67,6 +70,9 @@ void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) .skip = 3, }; + if (unlikely(!page_ext)) + return; + save_stack_trace(&trace); page_ext->order = order; @@ -79,6 +85,12 @@ void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) gfp_t __get_page_owner_gfp(struct page *page) { struct page_ext *page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + /* + * The caller just returns 0 if no valid gfp + * So return 0 here too. + */ + return 0; return page_ext->gfp_mask; } @@ -194,6 +206,8 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos) } page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* * Some pages could be missed by concurrent allocation or free, @@ -257,6 +271,8 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone) continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* Maybe overraping zone */ if (test_bit(PAGE_EXT_OWNER, &page_ext->flags)) diff --git a/mm/vmstat.c b/mm/vmstat.c index c54fd2924f25..c344e3609c53 100644 --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -1091,6 +1091,8 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m, continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags)) continue; -- 2.15.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH stable-4.4 2/2] mm/page_ext.c: check if page_ext is not prepared 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko @ 2017-11-22 12:09 ` Michal Hocko 2017-11-22 14:04 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Greg KH ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Michal Hocko @ 2017-11-22 12:09 UTC (permalink / raw) To: Greg KH Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Vlastimil Babka From: Jaewon Kim <jaewon31.kim@samsung.com> commit e492080e640c2d1235ddf3441cae634cfffef7e1 upstream. online_page_ext() and page_ext_init() allocate page_ext for each section, but they do not allocate if the first PFN is !pfn_present(pfn) or !pfn_valid(pfn). Then section->page_ext remains as NULL. lookup_page_ext checks NULL only if CONFIG_DEBUG_VM is enabled. For a valid PFN, __set_page_owner will try to get page_ext through lookup_page_ext. Without CONFIG_DEBUG_VM lookup_page_ext will misuse NULL pointer as value 0. This incurrs invalid address access. This is the panic example when PFN 0x100000 is not valid but PFN 0x13FC00 is being used for page_ext. section->page_ext is NULL, get_entry returned invalid page_ext address as 0x1DFA000 for a PFN 0x13FC00. To avoid this panic, CONFIG_DEBUG_VM should be removed so that page_ext will be checked at all times. Unable to handle kernel paging request at virtual address 01dfa014 ------------[ cut here ]------------ Kernel BUG at ffffff80082371e0 [verbose debug info unavailable] Internal error: Oops: 96000045 [#1] PREEMPT SMP Modules linked in: PC is at __set_page_owner+0x48/0x78 LR is at __set_page_owner+0x44/0x78 __set_page_owner+0x48/0x78 get_page_from_freelist+0x880/0x8e8 __alloc_pages_nodemask+0x14c/0xc48 __do_page_cache_readahead+0xdc/0x264 filemap_fault+0x2ac/0x550 ext4_filemap_fault+0x3c/0x58 __do_fault+0x80/0x120 handle_mm_fault+0x704/0xbb0 do_page_fault+0x2e8/0x394 do_mem_abort+0x88/0x124 Pre-4.7 kernels also need commit f86e4271978b ("mm: check the return value of lookup_page_ext for all call sites"). Link: http://lkml.kernel.org/r/20171107094131.14621-1-jaewon31.kim@samsung.com Fixes: eefa864b701d ("mm/page_ext: resurrect struct page extending code for debugging") Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com> Acked-by: Michal Hocko <mhocko@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <js1304@gmail.com> Cc: <stable@vger.kernel.org> [depends on f86e427197, see above] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Michal Hocko <mhocko@suse.com> --- mm/page_ext.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mm/page_ext.c b/mm/page_ext.c index 292ca7b8debd..4d1eac0d4fc5 100644 --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -106,7 +106,6 @@ struct page_ext *lookup_page_ext(struct page *page) struct page_ext *base; base = NODE_DATA(page_to_nid(page))->node_page_ext; -#ifdef CONFIG_DEBUG_VM /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -115,7 +114,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (unlikely(!base)) return NULL; -#endif offset = pfn - round_down(node_start_pfn(page_to_nid(page)), MAX_ORDER_NR_PAGES); return base + offset; @@ -180,7 +178,6 @@ struct page_ext *lookup_page_ext(struct page *page) { unsigned long pfn = page_to_pfn(page); struct mem_section *section = __pfn_to_section(pfn); -#ifdef CONFIG_DEBUG_VM /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -189,7 +186,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (!section->page_ext) return NULL; -#endif return section->page_ext + pfn; } -- 2.15.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 2/2] mm/page_ext.c: check if page_ext is not prepared Michal Hocko @ 2017-11-22 14:04 ` Greg KH 2017-11-24 9:17 ` Jiri Slaby 2017-12-05 16:16 ` [PATCH stable-4.4 1/2] " Ben Hutchings 3 siblings, 0 replies; 18+ messages in thread From: Greg KH @ 2017-11-22 14:04 UTC (permalink / raw) To: Michal Hocko Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Wed, Nov 22, 2017 at 01:09:36PM +0100, Michal Hocko wrote: > From: Yang Shi <yang.shi@linaro.org> > > commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. > > Per the discussion with Joonsoo Kim [1], we need check the return value > of lookup_page_ext() for all call sites since it might return NULL in > some cases, although it is unlikely, i.e. memory hotplug. > > Tested with ltp with "page_owner=0". > > [1] http://lkml.kernel.org/r/20160519002809.GA10245@js1304-P5Q-DELUXE > > [akpm@linux-foundation.org: fix build-breaking typos] > [arnd@arndb.de: fix build problems from lookup_page_ext] > Link: http://lkml.kernel.org/r/6285269.2CksypHdYp@wuerfel > [akpm@linux-foundation.org: coding-style fixes] > Link: http://lkml.kernel.org/r/1464023768-31025-1-git-send-email-yang.shi@linaro.org > Signed-off-by: Yang Shi <yang.shi@linaro.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Michal Hocko <mhocko@suse.com> Thanks for both of these! greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 2/2] mm/page_ext.c: check if page_ext is not prepared Michal Hocko 2017-11-22 14:04 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Greg KH @ 2017-11-24 9:17 ` Jiri Slaby 2017-11-24 9:28 ` Michal Hocko 2017-12-05 16:16 ` [PATCH stable-4.4 1/2] " Ben Hutchings 3 siblings, 1 reply; 18+ messages in thread From: Jiri Slaby @ 2017-11-24 9:17 UTC (permalink / raw) To: Michal Hocko, Greg KH Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On 11/22/2017, 01:09 PM, Michal Hocko wrote: > --- a/mm/debug-pagealloc.c > +++ b/mm/debug-pagealloc.c > @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return; > __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > } > > @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return; > __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > } > > @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return false; > return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); Now I am confused, your SLE12-SP2's backport in patches.fixes/0001-mm-check-the-return-value-of-lookup_page_ext-for-all.patch does the opposite in all three: + if (!page_ext) + return; thanks, -- js suse labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:17 ` Jiri Slaby @ 2017-11-24 9:28 ` Michal Hocko 2017-11-24 9:29 ` Jiri Slaby 2017-11-24 9:30 ` [PATCH] " Michal Hocko 0 siblings, 2 replies; 18+ messages in thread From: Michal Hocko @ 2017-11-24 9:28 UTC (permalink / raw) To: Jiri Slaby Cc: Greg KH, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Fri 24-11-17 10:17:58, Jiri Slaby wrote: > On 11/22/2017, 01:09 PM, Michal Hocko wrote: > > --- a/mm/debug-pagealloc.c > > +++ b/mm/debug-pagealloc.c > > @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return; > > __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > } > > > > @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return; > > __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > } > > > > @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return false; > > return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > Now I am confused, your SLE12-SP2's backport in > patches.fixes/0001-mm-check-the-return-value-of-lookup_page_ext-for-all.patch > does the opposite in all three: > + if (!page_ext) > + return; Dohh, because I screwed up! Thanks for catching this. I will repost the fixed patch as a reply to this email. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:28 ` Michal Hocko @ 2017-11-24 9:29 ` Jiri Slaby 2017-11-24 9:39 ` Michal Hocko 2017-11-24 9:30 ` [PATCH] " Michal Hocko 1 sibling, 1 reply; 18+ messages in thread From: Jiri Slaby @ 2017-11-24 9:29 UTC (permalink / raw) To: Michal Hocko Cc: Greg KH, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On 11/24/2017, 10:28 AM, Michal Hocko wrote: > I will repost the fixed patch as a reply to this email. Since 4.4.101 was already released, you should send a bare fix instead then :). -- js suse labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:29 ` Jiri Slaby @ 2017-11-24 9:39 ` Michal Hocko 2017-11-24 10:14 ` Greg KH 0 siblings, 1 reply; 18+ messages in thread From: Michal Hocko @ 2017-11-24 9:39 UTC (permalink / raw) To: Jiri Slaby, Greg KH Cc: jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Fri 24-11-17 10:29:41, Jiri Slaby wrote: > On 11/24/2017, 10:28 AM, Michal Hocko wrote: > > I will repost the fixed patch as a reply to this email. > > Since 4.4.101 was already released, you should send a bare fix instead > then :). Sigh... Greg, could you queue this one up then? I am really sorry about the screw up. The rest of the backport should be ok. --- commit 5dfbfb99a64d1554eac7e3074af49e39bd104c35 Author: Michal Hocko <mhocko@suse.com> Date: Fri Nov 24 10:34:07 2017 +0100 mm, hwpoison: fixup "mm: check the return value of lookup_page_ext for all call sites" Backport of the upstream commit f86e4271978b ("mm: check the return value of lookup_page_ext for all call sites") is wrong for hwpoison pages. I have accidentally negated the condition for bailout. This basically disables hwpoison pages tracking while the code still might crash on unusual configurations when struct pages do not have page_ext allocated. The fix is trivial to invert the condition. Reported-by: Jiri Slaby <jslaby@suse.cz> Signed-off-by: Michal Hocko <mhocko@suse.com> diff --git a/mm/debug-pagealloc.c b/mm/debug-pagealloc.c index fe1c61f7cf26..3b8f1b83610e 100644 --- a/mm/debug-pagealloc.c +++ b/mm/debug-pagealloc.c @@ -34,7 +34,7 @@ static inline void set_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); - if (page_ext) + if (!page_ext) return; __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -44,7 +44,7 @@ static inline void clear_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); - if (page_ext) + if (!page_ext) return; __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -54,7 +54,7 @@ static inline bool page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); - if (page_ext) + if (!page_ext) return false; return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } -- Michal Hocko SUSE Labs ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:39 ` Michal Hocko @ 2017-11-24 10:14 ` Greg KH 0 siblings, 0 replies; 18+ messages in thread From: Greg KH @ 2017-11-24 10:14 UTC (permalink / raw) To: Michal Hocko Cc: Jiri Slaby, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Fri, Nov 24, 2017 at 10:39:57AM +0100, Michal Hocko wrote: > On Fri 24-11-17 10:29:41, Jiri Slaby wrote: > > On 11/24/2017, 10:28 AM, Michal Hocko wrote: > > > I will repost the fixed patch as a reply to this email. > > > > Since 4.4.101 was already released, you should send a bare fix instead > > then :). > > Sigh... Greg, could you queue this one up then? I am really sorry about > the screw up. The rest of the backport should be ok. Not a problem, thanks for the fixup, I'll go run it through my build systems and then release a 4.4.102 with it. thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:28 ` Michal Hocko 2017-11-24 9:29 ` Jiri Slaby @ 2017-11-24 9:30 ` Michal Hocko 2017-11-24 9:39 ` Greg KH 1 sibling, 1 reply; 18+ messages in thread From: Michal Hocko @ 2017-11-24 9:30 UTC (permalink / raw) To: Greg KH Cc: Jiri Slaby, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim, Michal Hocko From: Yang Shi <yang.shi@linaro.org> commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. Per the discussion with Joonsoo Kim [1], we need check the return value of lookup_page_ext() for all call sites since it might return NULL in some cases, although it is unlikely, i.e. memory hotplug. Tested with ltp with "page_owner=0". [1] http://lkml.kernel.org/r/20160519002809.GA10245@js1304-P5Q-DELUXE [akpm@linux-foundation.org: fix build-breaking typos] [arnd@arndb.de: fix build problems from lookup_page_ext] Link: http://lkml.kernel.org/r/6285269.2CksypHdYp@wuerfel [akpm@linux-foundation.org: coding-style fixes] Link: http://lkml.kernel.org/r/1464023768-31025-1-git-send-email-yang.shi@linaro.org Signed-off-by: Yang Shi <yang.shi@linaro.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Michal Hocko <mhocko@suse.com> --- include/linux/page_idle.h | 43 ++++++++++++++++++++++++++++++++++++------- mm/debug-pagealloc.c | 6 ++++++ mm/page_alloc.c | 6 ++++++ mm/page_owner.c | 16 ++++++++++++++++ mm/vmstat.c | 2 ++ 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/include/linux/page_idle.h b/include/linux/page_idle.h index bf268fa92c5b..fec40271339f 100644 --- a/include/linux/page_idle.h +++ b/include/linux/page_idle.h @@ -46,33 +46,62 @@ extern struct page_ext_operations page_idle_ops; static inline bool page_is_young(struct page *page) { - return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline void set_page_young(struct page *page) { - set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool test_and_clear_page_young(struct page *page) { - return test_and_clear_bit(PAGE_EXT_YOUNG, - &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_and_clear_bit(PAGE_EXT_YOUNG, &page_ext->flags); } static inline bool page_is_idle(struct page *page) { - return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return false; + + return test_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void set_page_idle(struct page *page) { - set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + set_bit(PAGE_EXT_IDLE, &page_ext->flags); } static inline void clear_page_idle(struct page *page) { - clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags); + struct page_ext *page_ext = lookup_page_ext(page); + + if (unlikely(!page_ext)) + return; + + clear_bit(PAGE_EXT_IDLE, &page_ext->flags); } #endif /* CONFIG_64BIT */ diff --git a/mm/debug-pagealloc.c b/mm/debug-pagealloc.c index 5bf5906ce13b..3b8f1b83610e 100644 --- a/mm/debug-pagealloc.c +++ b/mm/debug-pagealloc.c @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (!page_ext) + return; __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (!page_ext) + return; __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) struct page_ext *page_ext; page_ext = lookup_page_ext(page); + if (!page_ext) + return false; return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); } diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 6b5421ae86c6..38aca81deeaf 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -560,6 +560,9 @@ static inline void set_page_guard(struct zone *zone, struct page *page, return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __set_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); INIT_LIST_HEAD(&page->lru); @@ -577,6 +580,9 @@ static inline void clear_page_guard(struct zone *zone, struct page *page, return; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + return; + __clear_bit(PAGE_EXT_DEBUG_GUARD, &page_ext->flags); set_page_private(page, 0); diff --git a/mm/page_owner.c b/mm/page_owner.c index 983c3a10fa07..dd6b9cebf981 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -53,6 +53,8 @@ void __reset_page_owner(struct page *page, unsigned int order) for (i = 0; i < (1 << order); i++) { page_ext = lookup_page_ext(page + i); + if (unlikely(!page_ext)) + continue; __clear_bit(PAGE_EXT_OWNER, &page_ext->flags); } } @@ -60,6 +62,7 @@ void __reset_page_owner(struct page *page, unsigned int order) void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) { struct page_ext *page_ext = lookup_page_ext(page); + struct stack_trace trace = { .nr_entries = 0, .max_entries = ARRAY_SIZE(page_ext->trace_entries), @@ -67,6 +70,9 @@ void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) .skip = 3, }; + if (unlikely(!page_ext)) + return; + save_stack_trace(&trace); page_ext->order = order; @@ -79,6 +85,12 @@ void __set_page_owner(struct page *page, unsigned int order, gfp_t gfp_mask) gfp_t __get_page_owner_gfp(struct page *page) { struct page_ext *page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + /* + * The caller just returns 0 if no valid gfp + * So return 0 here too. + */ + return 0; return page_ext->gfp_mask; } @@ -194,6 +206,8 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos) } page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* * Some pages could be missed by concurrent allocation or free, @@ -257,6 +271,8 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone) continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; /* Maybe overraping zone */ if (test_bit(PAGE_EXT_OWNER, &page_ext->flags)) diff --git a/mm/vmstat.c b/mm/vmstat.c index c54fd2924f25..c344e3609c53 100644 --- a/mm/vmstat.c +++ b/mm/vmstat.c @@ -1091,6 +1091,8 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m, continue; page_ext = lookup_page_ext(page); + if (unlikely(!page_ext)) + continue; if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags)) continue; -- 2.15.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] mm: check the return value of lookup_page_ext for all call sites 2017-11-24 9:30 ` [PATCH] " Michal Hocko @ 2017-11-24 9:39 ` Greg KH 0 siblings, 0 replies; 18+ messages in thread From: Greg KH @ 2017-11-24 9:39 UTC (permalink / raw) To: Michal Hocko Cc: Jiri Slaby, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim, Michal Hocko On Fri, Nov 24, 2017 at 10:30:32AM +0100, Michal Hocko wrote: > From: Yang Shi <yang.shi@linaro.org> > > commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. > > Per the discussion with Joonsoo Kim [1], we need check the return value > of lookup_page_ext() for all call sites since it might return NULL in > some cases, although it is unlikely, i.e. memory hotplug. > > Tested with ltp with "page_owner=0". > > [1] http://lkml.kernel.org/r/20160519002809.GA10245@js1304-P5Q-DELUXE > > [akpm@linux-foundation.org: fix build-breaking typos] > [arnd@arndb.de: fix build problems from lookup_page_ext] > Link: http://lkml.kernel.org/r/6285269.2CksypHdYp@wuerfel > [akpm@linux-foundation.org: coding-style fixes] > Link: http://lkml.kernel.org/r/1464023768-31025-1-git-send-email-yang.shi@linaro.org > Signed-off-by: Yang Shi <yang.shi@linaro.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Michal Hocko <mhocko@suse.com> Can you send a patch on top of 4.4.101 to resolve this instead? thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko ` (2 preceding siblings ...) 2017-11-24 9:17 ` Jiri Slaby @ 2017-12-05 16:16 ` Ben Hutchings 2017-12-05 16:17 ` Ben Hutchings 2017-12-05 19:16 ` Michal Hocko 3 siblings, 2 replies; 18+ messages in thread From: Ben Hutchings @ 2017-12-05 16:16 UTC (permalink / raw) To: Michal Hocko, Greg KH Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Wed, 2017-11-22 at 13:09 +0100, Michal Hocko wrote: > From: Yang Shi <yang.shi@linaro.org> > > commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. > > Per the discussion with Joonsoo Kim [1], we need check the return value > of lookup_page_ext() for all call sites since it might return NULL in > some cases, although it is unlikely, i.e. memory hotplug. > > Tested with ltp with "page_owner=0". [...] > --- a/mm/debug-pagealloc.c > +++ b/mm/debug-pagealloc.c > @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page > *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return; This, and the other checks added to debug-pagealloc.c, are reversed. (This is specific to the 4.4 backport - in the upstream version these functions are in mm/page_poison.c and were patched correctly.) Ben. > __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > } > > @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page > *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return; > __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > } > > @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) > struct page_ext *page_ext; > > page_ext = lookup_page_ext(page); > + if (page_ext) > + return false; > return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > } > [...] -- Ben Hutchings Software Developer, Codethink Ltd. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-12-05 16:16 ` [PATCH stable-4.4 1/2] " Ben Hutchings @ 2017-12-05 16:17 ` Ben Hutchings 2017-12-05 19:16 ` Michal Hocko 1 sibling, 0 replies; 18+ messages in thread From: Ben Hutchings @ 2017-12-05 16:17 UTC (permalink / raw) To: Michal Hocko, Greg KH Cc: jaewon31.kim, akpm, js1304, mhocko, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim Sorry, I see this got fixed already. Ben. On Tue, 2017-12-05 at 16:16 +0000, Ben Hutchings wrote: > On Wed, 2017-11-22 at 13:09 +0100, Michal Hocko wrote: > > From: Yang Shi <yang.shi@linaro.org> > > > > commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. > > > > Per the discussion with Joonsoo Kim [1], we need check the return value > > of lookup_page_ext() for all call sites since it might return NULL in > > some cases, although it is unlikely, i.e. memory hotplug. > > > > Tested with ltp with "page_owner=0". > > [...] > > --- a/mm/debug-pagealloc.c > > +++ b/mm/debug-pagealloc.c > > @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page > > *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return; > > This, and the other checks added to debug-pagealloc.c, are reversed. > (This is specific to the 4.4 backport - in the upstream version these > functions are in mm/page_poison.c and were patched correctly.) > > Ben. > > > __set_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > } > > > > @@ -42,6 +44,8 @@ static inline void clear_page_poison(struct page > > *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return; > > __clear_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > } > > > > @@ -50,6 +54,8 @@ static inline bool page_poison(struct page *page) > > struct page_ext *page_ext; > > > > page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return false; > > return test_bit(PAGE_EXT_DEBUG_POISON, &page_ext->flags); > > } > > > > [...] > -- Ben Hutchings Software Developer, Codethink Ltd. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites 2017-12-05 16:16 ` [PATCH stable-4.4 1/2] " Ben Hutchings 2017-12-05 16:17 ` Ben Hutchings @ 2017-12-05 19:16 ` Michal Hocko 1 sibling, 0 replies; 18+ messages in thread From: Michal Hocko @ 2017-12-05 19:16 UTC (permalink / raw) To: Ben Hutchings Cc: Greg KH, jaewon31.kim, akpm, js1304, minchan, stable, torvalds, Yang Shi, Arnd Bergmann, Joonsoo Kim On Tue 05-12-17 16:16:35, Ben Hutchings wrote: > On Wed, 2017-11-22 at 13:09 +0100, Michal Hocko wrote: > > From: Yang Shi <yang.shi@linaro.org> > > > > commit f86e4271978bd93db466d6a95dad4b0fdcdb04f6 upstream. > > > > Per the discussion with Joonsoo Kim [1], we need check the return value > > of lookup_page_ext() for all call sites since it might return NULL in > > some cases, although it is unlikely, i.e.��memory hotplug. > > > > Tested with ltp with "page_owner=0". > [...] > > --- a/mm/debug-pagealloc.c > > +++ b/mm/debug-pagealloc.c > > @@ -34,6 +34,8 @@ static inline void set_page_poison(struct page > > *page) > > � struct page_ext *page_ext; > > � > > � page_ext = lookup_page_ext(page); > > + if (page_ext) > > + return; > > This, and the other checks added to debug-pagealloc.c, are reversed. > (This is specific to the 4.4 backport - in the upstream version these > functions are in mm/page_poison.c and were patched correctly.) Yes, I've sent a fixup and Greg has queued it up as 0208fabf7256245125fbabf03207a0da4000ea2d -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 18+ messages in thread
* FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree @ 2017-11-22 8:36 gregkh 0 siblings, 0 replies; 18+ messages in thread From: gregkh @ 2017-11-22 8:36 UTC (permalink / raw) To: jaewon31.kim; +Cc: stable The patch below does not apply to the 4.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable@vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ >From e492080e640c2d1235ddf3441cae634cfffef7e1 Mon Sep 17 00:00:00 2001 From: Jaewon Kim <jaewon31.kim@samsung.com> Date: Wed, 15 Nov 2017 17:39:07 -0800 Subject: [PATCH] mm/page_ext.c: check if page_ext is not prepared online_page_ext() and page_ext_init() allocate page_ext for each section, but they do not allocate if the first PFN is !pfn_present(pfn) or !pfn_valid(pfn). Then section->page_ext remains as NULL. lookup_page_ext checks NULL only if CONFIG_DEBUG_VM is enabled. For a valid PFN, __set_page_owner will try to get page_ext through lookup_page_ext. Without CONFIG_DEBUG_VM lookup_page_ext will misuse NULL pointer as value 0. This incurrs invalid address access. This is the panic example when PFN 0x100000 is not valid but PFN 0x13FC00 is being used for page_ext. section->page_ext is NULL, get_entry returned invalid page_ext address as 0x1DFA000 for a PFN 0x13FC00. To avoid this panic, CONFIG_DEBUG_VM should be removed so that page_ext will be checked at all times. Unable to handle kernel paging request at virtual address 01dfa014 ------------[ cut here ]------------ Kernel BUG at ffffff80082371e0 [verbose debug info unavailable] Internal error: Oops: 96000045 [#1] PREEMPT SMP Modules linked in: PC is at __set_page_owner+0x48/0x78 LR is at __set_page_owner+0x44/0x78 __set_page_owner+0x48/0x78 get_page_from_freelist+0x880/0x8e8 __alloc_pages_nodemask+0x14c/0xc48 __do_page_cache_readahead+0xdc/0x264 filemap_fault+0x2ac/0x550 ext4_filemap_fault+0x3c/0x58 __do_fault+0x80/0x120 handle_mm_fault+0x704/0xbb0 do_page_fault+0x2e8/0x394 do_mem_abort+0x88/0x124 Pre-4.7 kernels also need commit f86e4271978b ("mm: check the return value of lookup_page_ext for all call sites"). Link: http://lkml.kernel.org/r/20171107094131.14621-1-jaewon31.kim@samsung.com Fixes: eefa864b701d ("mm/page_ext: resurrect struct page extending code for debugging") Signed-off-by: Jaewon Kim <jaewon31.kim@samsung.com> Acked-by: Michal Hocko <mhocko@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Minchan Kim <minchan@kernel.org> Cc: Joonsoo Kim <js1304@gmail.com> Cc: <stable@vger.kernel.org> [depends on f86e427197, see above] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> diff --git a/mm/page_ext.c b/mm/page_ext.c index 4f0367d472c4..2c16216c29b6 100644 --- a/mm/page_ext.c +++ b/mm/page_ext.c @@ -125,7 +125,6 @@ struct page_ext *lookup_page_ext(struct page *page) struct page_ext *base; base = NODE_DATA(page_to_nid(page))->node_page_ext; -#if defined(CONFIG_DEBUG_VM) /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -134,7 +133,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (unlikely(!base)) return NULL; -#endif index = pfn - round_down(node_start_pfn(page_to_nid(page)), MAX_ORDER_NR_PAGES); return get_entry(base, index); @@ -199,7 +197,6 @@ struct page_ext *lookup_page_ext(struct page *page) { unsigned long pfn = page_to_pfn(page); struct mem_section *section = __pfn_to_section(pfn); -#if defined(CONFIG_DEBUG_VM) /* * The sanity checks the page allocator does upon freeing a * page can reach here before the page_ext arrays are @@ -208,7 +205,6 @@ struct page_ext *lookup_page_ext(struct page *page) */ if (!section->page_ext) return NULL; -#endif return get_entry(section->page_ext, pfn); } ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-12-05 19:17 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-22 8:37 FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree gregkh 2017-11-22 9:47 ` Michal Hocko 2017-11-22 9:54 ` Greg KH 2017-11-22 12:08 ` Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Michal Hocko 2017-11-22 12:09 ` [PATCH stable-4.4 2/2] mm/page_ext.c: check if page_ext is not prepared Michal Hocko 2017-11-22 14:04 ` [PATCH stable-4.4 1/2] mm: check the return value of lookup_page_ext for all call sites Greg KH 2017-11-24 9:17 ` Jiri Slaby 2017-11-24 9:28 ` Michal Hocko 2017-11-24 9:29 ` Jiri Slaby 2017-11-24 9:39 ` Michal Hocko 2017-11-24 10:14 ` Greg KH 2017-11-24 9:30 ` [PATCH] " Michal Hocko 2017-11-24 9:39 ` Greg KH 2017-12-05 16:16 ` [PATCH stable-4.4 1/2] " Ben Hutchings 2017-12-05 16:17 ` Ben Hutchings 2017-12-05 19:16 ` Michal Hocko -- strict thread matches above, loose matches on Subject: below -- 2017-11-22 8:36 FAILED: patch "[PATCH] mm/page_ext.c: check if page_ext is not prepared" failed to apply to 4.4-stable tree gregkh
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).