On Fri, Mar 31, 2017 at 2:33 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 31 Mar 2017 09:40:28 -0700 Kees Cook <keescook@xxxxxxxxxxxx> wrote: > >> As found in PaX, this adds a cheap check on heap consistency, just to >> notice if things have gotten corrupted in the page lookup. > > "As found in PaX" isn't a very illuminating justification for such a > change. Was there a real kernel bug which this would have exposed, or > what? I don't know off the top of my head, but given the kinds of heap attacks I've been seeing, I think this added consistency check is worth it given how inexpensive it is. When heap metadata gets corrupted, we can get into nasty side-effects that can be attacker-controlled, so better to catch obviously bad states as early as possible. >> --- a/mm/slab.h >> +++ b/mm/slab.h >> @@ -384,6 +384,7 @@ static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x) >> return s; >> >> page = virt_to_head_page(x); >> + BUG_ON(!PageSlab(page)); >> cachep = page->slab_cache; >> if (slab_equal_or_root(cachep, s)) >> return cachep; > > BUG_ON might be too severe. I expect the kindest VM_WARN_ON_ONCE() > would suffice here, but without more details it is hard to say. So, WARN isn't enough to protect the kernel (execution continues and the memory is still dereferenced for malicious purposes, etc). Perhaps use CHECK_DATA_CORRUPTION() here, which can either WARN and take a "safe" path, or BUG (depending on config paranoia of the builder). I've got a series adding it in a number of other places, so I could add this patch to that series? -Kees -- Kees Cook Pixel Security -- 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>
