From: Nick Piggin <npiggin@kernel.dk>
To: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Subject: [patch 02/28] kernel: kmem_ptr_validate considered harmful
Date: Wed, 17 Nov 2010 01:09:02 +1100 [thread overview]
Message-ID: <20101116142028.362270680@kernel.dk> (raw)
In-Reply-To: 20101116140900.039761100@kernel.dk
[-- Attachment #1: dcache-d_validate-broken.patch --]
[-- Type: text/plain, Size: 5805 bytes --]
This is a nasty ugly and error prone API. It's sole user, dcache, could not
get it right so there is roughly zero chance that anything else will.
Signed-off-by: Nick Piggin <npiggin@kernel.dk>
---
include/linux/slab.h | 2 --
mm/slab.c | 32 +-------------------------------
mm/slob.c | 5 -----
mm/slub.c | 40 ----------------------------------------
mm/util.c | 21 ---------------------
5 files changed, 1 insertion(+), 99 deletions(-)
Index: linux-2.6/include/linux/slab.h
===================================================================
--- linux-2.6.orig/include/linux/slab.h 2010-11-17 00:10:41.000000000 +1100
+++ linux-2.6/include/linux/slab.h 2010-11-17 00:11:49.000000000 +1100
@@ -106,8 +106,6 @@ int kmem_cache_shrink(struct kmem_cache
void kmem_cache_free(struct kmem_cache *, void *);
unsigned int kmem_cache_size(struct kmem_cache *);
const char *kmem_cache_name(struct kmem_cache *);
-int kern_ptr_validate(const void *ptr, unsigned long size);
-int kmem_ptr_validate(struct kmem_cache *cachep, const void *ptr);
/*
* Please use this macro to create slab caches. Simply specify the
Index: linux-2.6/mm/slab.c
===================================================================
--- linux-2.6.orig/mm/slab.c 2010-11-17 00:10:41.000000000 +1100
+++ linux-2.6/mm/slab.c 2010-11-17 00:11:49.000000000 +1100
@@ -2781,7 +2781,7 @@ static void slab_put_obj(struct kmem_cac
/*
* Map pages beginning at addr to the given cache and slab. This is required
* for the slab allocator to be able to lookup the cache and slab of a
- * virtual address for kfree, ksize, kmem_ptr_validate, and slab debugging.
+ * virtual address for kfree, ksize, and slab debugging.
*/
static void slab_map_pages(struct kmem_cache *cache, struct slab *slab,
void *addr)
@@ -3660,36 +3660,6 @@ void *kmem_cache_alloc_notrace(struct km
EXPORT_SYMBOL(kmem_cache_alloc_notrace);
#endif
-/**
- * kmem_ptr_validate - check if an untrusted pointer might be a slab entry.
- * @cachep: the cache we're checking against
- * @ptr: pointer to validate
- *
- * This verifies that the untrusted pointer looks sane;
- * it is _not_ a guarantee that the pointer is actually
- * part of the slab cache in question, but it at least
- * validates that the pointer can be dereferenced and
- * looks half-way sane.
- *
- * Currently only used for dentry validation.
- */
-int kmem_ptr_validate(struct kmem_cache *cachep, const void *ptr)
-{
- unsigned long size = cachep->buffer_size;
- struct page *page;
-
- if (unlikely(!kern_ptr_validate(ptr, size)))
- goto out;
- page = virt_to_page(ptr);
- if (unlikely(!PageSlab(page)))
- goto out;
- if (unlikely(page_get_cache(page) != cachep))
- goto out;
- return 1;
-out:
- return 0;
-}
-
#ifdef CONFIG_NUMA
void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
{
Index: linux-2.6/mm/slub.c
===================================================================
--- linux-2.6.orig/mm/slub.c 2010-11-17 00:10:41.000000000 +1100
+++ linux-2.6/mm/slub.c 2010-11-17 00:24:15.000000000 +1100
@@ -1917,17 +1917,6 @@ void kmem_cache_free(struct kmem_cache *
}
EXPORT_SYMBOL(kmem_cache_free);
-/* Figure out on which slab page the object resides */
-static struct page *get_object_page(const void *x)
-{
- struct page *page = virt_to_head_page(x);
-
- if (!PageSlab(page))
- return NULL;
-
- return page;
-}
-
/*
* Object placement in a slab is made very easy because we always start at
* offset 0. If we tune the size of the object to the alignment then we can
@@ -2386,35 +2375,6 @@ static int kmem_cache_open(struct kmem_c
}
/*
- * Check if a given pointer is valid
- */
-int kmem_ptr_validate(struct kmem_cache *s, const void *object)
-{
- struct page *page;
-
- if (!kern_ptr_validate(object, s->size))
- return 0;
-
- page = get_object_page(object);
-
- if (!page || s != page->slab)
- /* No slab or wrong slab */
- return 0;
-
- if (!check_valid_pointer(s, page, object))
- return 0;
-
- /*
- * We could also check if the object is on the slabs freelist.
- * But this would be too expensive and it seems that the main
- * purpose of kmem_ptr_valid() is to check if the object belongs
- * to a certain slab.
- */
- return 1;
-}
-EXPORT_SYMBOL(kmem_ptr_validate);
-
-/*
* Determine the size of a slab object
*/
unsigned int kmem_cache_size(struct kmem_cache *s)
Index: linux-2.6/mm/util.c
===================================================================
--- linux-2.6.orig/mm/util.c 2010-11-17 00:10:41.000000000 +1100
+++ linux-2.6/mm/util.c 2010-11-17 00:11:49.000000000 +1100
@@ -186,27 +186,6 @@ void kzfree(const void *p)
}
EXPORT_SYMBOL(kzfree);
-int kern_ptr_validate(const void *ptr, unsigned long size)
-{
- unsigned long addr = (unsigned long)ptr;
- unsigned long min_addr = PAGE_OFFSET;
- unsigned long align_mask = sizeof(void *) - 1;
-
- if (unlikely(addr < min_addr))
- goto out;
- if (unlikely(addr > (unsigned long)high_memory - size))
- goto out;
- if (unlikely(addr & align_mask))
- goto out;
- if (unlikely(!kern_addr_valid(addr)))
- goto out;
- if (unlikely(!kern_addr_valid(addr + size - 1)))
- goto out;
- return 1;
-out:
- return 0;
-}
-
/*
* strndup_user - duplicate an existing string from user space
* @s: The string to duplicate
Index: linux-2.6/mm/slob.c
===================================================================
--- linux-2.6.orig/mm/slob.c 2010-11-17 00:10:41.000000000 +1100
+++ linux-2.6/mm/slob.c 2010-11-17 00:11:49.000000000 +1100
@@ -678,11 +678,6 @@ int kmem_cache_shrink(struct kmem_cache
}
EXPORT_SYMBOL(kmem_cache_shrink);
-int kmem_ptr_validate(struct kmem_cache *a, const void *b)
-{
- return 0;
-}
-
static unsigned int slob_ready __read_mostly;
int slab_is_available(void)
next prev parent reply other threads:[~2010-11-16 14:09 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-16 14:09 [patch 00/28] [rfc] dcache scaling part 1 Nick Piggin
2010-11-16 14:09 ` [patch 01/28] fs: d_validate fixes Nick Piggin
[not found] ` <20101116142028.254946611-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
2010-11-17 10:44 ` Andi Kleen
2010-11-18 20:51 ` David Miller
2010-11-18 20:59 ` David Miller
2010-11-19 5:05 ` Nick Piggin
2010-11-19 5:01 ` Nick Piggin
2010-11-16 14:09 ` Nick Piggin [this message]
2010-11-16 14:09 ` [patch 03/28] fs: dcache documentation cleanup Nick Piggin
2010-11-16 14:09 ` [patch 04/28] fs: change d_delete semantics Nick Piggin
2010-11-17 0:16 ` Tim Pepper
2010-11-16 14:09 ` [patch 05/28] cifs: dont overwrite dentry name in d_revalidate Nick Piggin
2010-11-16 14:09 ` [patch 06/28] jfs: " Nick Piggin
2010-11-16 14:09 ` [patch 07/28] fs: change d_compare for rcu-walk Nick Piggin
2010-11-17 0:44 ` Tim Pepper
2010-11-16 14:09 ` [patch 08/28] fs: change d_hash " Nick Piggin
2010-11-17 0:50 ` Tim Pepper
2010-11-16 14:09 ` [patch 09/28] hostfs: simplify locking Nick Piggin
2010-11-16 14:09 ` [patch 10/28] fs: dcache scale hash Nick Piggin
2010-11-16 14:09 ` [patch 11/28] fs: dcache scale lru Nick Piggin
2010-11-16 14:09 ` [patch 12/28] fs: dcache scale dentry refcount Nick Piggin
2010-11-16 14:09 ` [patch 13/28] fs: dcache scale d_unhashed Nick Piggin
2010-11-19 19:41 ` Tim Pepper
2010-11-16 14:09 ` [patch 14/28] fs: dcache scale subdirs Nick Piggin
2010-11-19 19:41 ` Tim Pepper
2010-11-16 14:09 ` [patch 15/28] fs: scale inode alias list Nick Piggin
2010-11-19 19:41 ` Tim Pepper
2010-11-16 14:09 ` [patch 16/28] fs: Use rename lock and RCU for multi-step operations Nick Piggin
2010-11-19 19:42 ` Tim Pepper
2010-11-16 14:09 ` [patch 17/28] fs: increase d_name lock coverage Nick Piggin
2010-11-16 14:09 ` [patch 18/28] fs: dcache remove dcache_lock Nick Piggin
2010-11-16 14:09 ` [patch 19/28] fs: dcache avoid starvation in dcache multi-step operations Nick Piggin
2010-11-16 14:09 ` [patch 20/28] fs: dcache reduce dput locking Nick Piggin
2010-11-16 14:09 ` [patch 21/28] fs: dcache reduce locking in d_alloc Nick Piggin
2010-11-16 14:09 ` [patch 22/28] fs: dcache reduce dcache_inode_lock Nick Piggin
2010-11-16 14:09 ` [patch 23/28] fs: dcache rationalise dget variants Nick Piggin
2010-11-16 14:09 ` [patch 24/28] fs: dcache reduce d_parent locking Nick Piggin
2010-11-16 14:09 ` [patch 25/28] fs: dcache reduce prune_one_dentry locking Nick Piggin
2010-11-16 14:09 ` [patch 26/28] fs: reduce dcache_inode_lock width in lru scanning Nick Piggin
2010-11-16 14:09 ` [patch 27/28] fs: use RCU in shrink_dentry_list to reduce lock nesting Nick Piggin
2010-11-16 14:09 ` [patch 28/28] fs: consolidate dentry kill sequence Nick Piggin
2010-11-17 2:12 ` [patch 00/28] [rfc] dcache scaling part 1 Dave Chinner
2010-11-17 10:56 ` Andi Kleen
2010-11-17 11:19 ` Nick Piggin
2010-11-17 12:01 ` Andi Kleen
2010-11-19 19:43 ` Tim Pepper
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=20101116142028.362270680@kernel.dk \
--to=npiggin@kernel.dk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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).