Commit 5bf5f03c271907978489868a4c72aeb42b5127d2
Committed by
Linus Torvalds
1 parent
dbda591d92
Exists in
master
and in
20 other branches
mm: fix slab->page flags corruption
Transparent huge pages can change page->flags (PG_compound_lock) without taking Slab lock. Since THP can not break slab pages we can safely access compound page without taking compound lock. Specifically this patch fixes a race between compound_unlock() and slab functions which perform page-flags updates. This can occur when get_page()/put_page() is called on a page from slab. [akpm@linux-foundation.org: tweak comment text, fix comment layout, fix label indenting] Reported-by: Amey Bhide <abhide@nicira.com> Signed-off-by: Pravin B Shelar <pshelar@nicira.com> Reviewed-by: Christoph Lameter <cl@linux.com> Acked-by: Andrea Arcangeli <aarcange@redhat.com> Cc: Pekka Enberg <penberg@cs.helsinki.fi> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 2 changed files with 37 additions and 2 deletions Side-by-side Diff
include/linux/mm.h
... | ... | @@ -321,6 +321,7 @@ |
321 | 321 | static inline void compound_lock(struct page *page) |
322 | 322 | { |
323 | 323 | #ifdef CONFIG_TRANSPARENT_HUGEPAGE |
324 | + VM_BUG_ON(PageSlab(page)); | |
324 | 325 | bit_spin_lock(PG_compound_lock, &page->flags); |
325 | 326 | #endif |
326 | 327 | } |
... | ... | @@ -328,6 +329,7 @@ |
328 | 329 | static inline void compound_unlock(struct page *page) |
329 | 330 | { |
330 | 331 | #ifdef CONFIG_TRANSPARENT_HUGEPAGE |
332 | + VM_BUG_ON(PageSlab(page)); | |
331 | 333 | bit_spin_unlock(PG_compound_lock, &page->flags); |
332 | 334 | #endif |
333 | 335 | } |
mm/swap.c
... | ... | @@ -82,7 +82,26 @@ |
82 | 82 | if (likely(page != page_head && |
83 | 83 | get_page_unless_zero(page_head))) { |
84 | 84 | unsigned long flags; |
85 | + | |
85 | 86 | /* |
87 | + * THP can not break up slab pages so avoid taking | |
88 | + * compound_lock(). Slab performs non-atomic bit ops | |
89 | + * on page->flags for better performance. In particular | |
90 | + * slab_unlock() in slub used to be a hot path. It is | |
91 | + * still hot on arches that do not support | |
92 | + * this_cpu_cmpxchg_double(). | |
93 | + */ | |
94 | + if (PageSlab(page_head)) { | |
95 | + if (PageTail(page)) { | |
96 | + if (put_page_testzero(page_head)) | |
97 | + VM_BUG_ON(1); | |
98 | + | |
99 | + atomic_dec(&page->_mapcount); | |
100 | + goto skip_lock_tail; | |
101 | + } else | |
102 | + goto skip_lock; | |
103 | + } | |
104 | + /* | |
86 | 105 | * page_head wasn't a dangling pointer but it |
87 | 106 | * may not be a head page anymore by the time |
88 | 107 | * we obtain the lock. That is ok as long as it |
89 | 108 | |
... | ... | @@ -92,10 +111,10 @@ |
92 | 111 | if (unlikely(!PageTail(page))) { |
93 | 112 | /* __split_huge_page_refcount run before us */ |
94 | 113 | compound_unlock_irqrestore(page_head, flags); |
95 | - VM_BUG_ON(PageHead(page_head)); | |
114 | +skip_lock: | |
96 | 115 | if (put_page_testzero(page_head)) |
97 | 116 | __put_single_page(page_head); |
98 | - out_put_single: | |
117 | +out_put_single: | |
99 | 118 | if (put_page_testzero(page)) |
100 | 119 | __put_single_page(page); |
101 | 120 | return; |
... | ... | @@ -115,6 +134,8 @@ |
115 | 134 | VM_BUG_ON(atomic_read(&page_head->_count) <= 0); |
116 | 135 | VM_BUG_ON(atomic_read(&page->_count) != 0); |
117 | 136 | compound_unlock_irqrestore(page_head, flags); |
137 | + | |
138 | +skip_lock_tail: | |
118 | 139 | if (put_page_testzero(page_head)) { |
119 | 140 | if (PageHead(page_head)) |
120 | 141 | __put_compound_page(page_head); |
... | ... | @@ -162,6 +183,18 @@ |
162 | 183 | struct page *page_head = compound_trans_head(page); |
163 | 184 | |
164 | 185 | if (likely(page != page_head && get_page_unless_zero(page_head))) { |
186 | + | |
187 | + /* Ref to put_compound_page() comment. */ | |
188 | + if (PageSlab(page_head)) { | |
189 | + if (likely(PageTail(page))) { | |
190 | + __get_page_tail_foll(page, false); | |
191 | + return true; | |
192 | + } else { | |
193 | + put_page(page_head); | |
194 | + return false; | |
195 | + } | |
196 | + } | |
197 | + | |
165 | 198 | /* |
166 | 199 | * page_head wasn't a dangling pointer but it |
167 | 200 | * may not be a head page anymore by the time |