From: Kees Cook <keescook@chromium.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>,
Matthew Wilcox <willy@infradead.org>,
Alexander Popov <alex.popov@linux.com>,
Alexander Potapenko <glider@google.com>,
Christoph Lameter <cl@linux.com>,
Pekka Enberg <penberg@kernel.org>,
David Rientjes <rientjes@google.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH 2/3] mm/slab: Sanity-check page type when looking up cache
Date: Wed, 29 May 2019 21:50:16 -0700 [thread overview]
Message-ID: <20190530045017.15252-3-keescook@chromium.org> (raw)
In-Reply-To: <20190530045017.15252-1-keescook@chromium.org>
This avoids any possible type confusion when looking up an object. For
example, if a non-slab were to be passed to kfree(), the invalid
slab_cache pointer (i.e. overlapped with some other value from the struct
page union) would be used for subsequent slab manipulations that could
lead to further memory corruption.
Since the page is already in cache, adding the PageSlab() check will
have nearly zero cost, so add a check and WARN() to virt_to_cache().
Additionally replaces an open-coded virt_to_cache(). To support the failure
mode this also updates all callers of virt_to_cache() and cache_from_obj()
to handle a NULL cache pointer return value (though note that several
already handle this case gracefully).
Signed-off-by: Kees Cook <keescook@chromium.org>
---
mm/slab.c | 14 +++++++-------
mm/slab.h | 17 +++++++++++++----
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/mm/slab.c b/mm/slab.c
index f7117ad9b3a3..9e3eee5568b6 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -371,12 +371,6 @@ static void **dbg_userword(struct kmem_cache *cachep, void *objp)
static int slab_max_order = SLAB_MAX_ORDER_LO;
static bool slab_max_order_set __initdata;
-static inline struct kmem_cache *virt_to_cache(const void *obj)
-{
- struct page *page = virt_to_head_page(obj);
- return page->slab_cache;
-}
-
static inline void *index_to_obj(struct kmem_cache *cache, struct page *page,
unsigned int idx)
{
@@ -3715,6 +3709,8 @@ void kmem_cache_free_bulk(struct kmem_cache *orig_s, size_t size, void **p)
s = virt_to_cache(objp);
else
s = cache_from_obj(orig_s, objp);
+ if (!s)
+ continue;
debug_check_no_locks_freed(objp, s->object_size);
if (!(s->flags & SLAB_DEBUG_OBJECTS))
@@ -3749,6 +3745,8 @@ void kfree(const void *objp)
local_irq_save(flags);
kfree_debugcheck(objp);
c = virt_to_cache(objp);
+ if (!c)
+ return;
debug_check_no_locks_freed(objp, c->object_size);
debug_check_no_obj_freed(objp, c->object_size);
@@ -4219,13 +4217,15 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page,
*/
size_t ksize(const void *objp)
{
+ struct kmem_cache *c;
size_t size;
BUG_ON(!objp);
if (unlikely(objp == ZERO_SIZE_PTR))
return 0;
- size = virt_to_cache(objp)->object_size;
+ c = virt_to_cache(objp);
+ size = c ? c->object_size : 0;
/* We assume that ksize callers could use the whole allocated area,
* so we need to unpoison this area.
*/
diff --git a/mm/slab.h b/mm/slab.h
index 4dafae2c8620..739099af6cbb 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -350,10 +350,20 @@ static inline void memcg_link_cache(struct kmem_cache *s)
#endif /* CONFIG_MEMCG_KMEM */
+static inline struct kmem_cache *virt_to_cache(const void *obj)
+{
+ struct page *page;
+
+ page = virt_to_head_page(obj);
+ if (WARN_ONCE(!PageSlab(page), "%s: Object is not a Slab page!\n",
+ __func__))
+ return NULL;
+ return page->slab_cache;
+}
+
static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x)
{
struct kmem_cache *cachep;
- struct page *page;
/*
* When kmemcg is not being used, both assignments should return the
@@ -367,9 +377,8 @@ static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x)
!unlikely(s->flags & SLAB_CONSISTENCY_CHECKS))
return s;
- page = virt_to_head_page(x);
- cachep = page->slab_cache;
- WARN_ONCE(!slab_equal_or_root(cachep, s),
+ cachep = virt_to_cache(x);
+ WARN_ONCE(cachep && !slab_equal_or_root(cachep, s),
"%s: Wrong slab cache. %s but object is from %s\n",
__func__, s->name, cachep->name);
return cachep;
--
2.17.1
next prev parent reply other threads:[~2019-05-30 4:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-30 4:50 [PATCH 0/3] mm/slab: Improved sanity checking Kees Cook
2019-05-30 4:50 ` [PATCH 1/3] mm/slab: Validate cache membership under freelist hardening Kees Cook
2019-05-30 4:50 ` Kees Cook [this message]
2019-05-30 4:50 ` [PATCH 3/3] lkdtm/heap: Add tests for " Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190530045017.15252-3-keescook@chromium.org \
--to=keescook@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=alex.popov@linux.com \
--cc=cl@linux.com \
--cc=glider@google.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).