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: 5807 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:23 UTC|newest]
Thread overview: 49+ 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
2010-11-17 10:44 ` Andi Kleen
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-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-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.