All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roel Kluin <roel.kluin@gmail.com>
To: lkml <linux-kernel@vger.kernel.org>
Subject: build bug on pointer type
Date: Fri, 13 Mar 2009 17:17:20 +0100	[thread overview]
Message-ID: <49BA8710.20808@gmail.com> (raw)

I was thinking about forcing a build bug if anyone attempts to kfree a
struct sk_buff*, like this:

#define PTR_OR_BB_ON_TYPE(x, type) (&x[sizeof(char[0 -			\
			__builtin_types_compatible_p(typeof(x), type)])])

and:

#define kfree(x) _kfree(PTR_OR_BB_ON_TYPE((x), struct sk_buff*))

with _kfree as the function that is currently kfree.

It appears to work in my test.c.

I also compile this with (defconfig) and there were several errors, which I
fixed, included in the patch below, and also increased sparse warnings, like
this

In file included from /home/roel/dnld/src/kernel/git/linux-2.6/include/linux/vmstat.h:5,
                 from /home/roel/dnld/src/kernel/git/linux-2.6/include/linux/mm.h:595,
                 from /home/roel/dnld/src/kernel/git/linux-2.6/kernel/exit.c:7:
/home/roel/dnld/src/kernel/git/linux-2.6/include/linux/percpu.h: In function 'percpu_free':
/home/roel/dnld/src/kernel/git/linux-2.6/include/linux/percpu.h:98: warning: dereferencing 'void *' pointer

This is mostly due to wrappers for kfree that take a void* argument. and can be
fixed after compiling with sparse by:

sed -n -r "s/^([^:]*):([^:]*):.*derefer.*$/\2{s\/^\(.*[^[:alnum:]_]\)kfree$s\\\\\\\\\(\/\\\\\\\\1_kfree\\\\\\\\\(\/} \1/p" ../logs/make_def_log_20090313164543 | while read l f; do sed -i -r  "$l" "$f"; done

My question, is there something fundamentally wrong with this?

Not yet meant for inclusion, and only compile tested.
---
 block/genhd.c                      |    2 +-
 drivers/firmware/dmi-id.c          |    2 +-
 drivers/gpu/drm/i915/intel_modes.c |    2 +-
 include/linux/kernel.h             |    5 +++++
 include/linux/slab.h               |    5 ++++-
 kernel/exit.c                      |    2 +-
 lib/kref.c                         |    2 +-
 mm/slab.c                          |    4 ++--
 mm/slob.c                          |    4 ++--
 mm/slub.c                          |    4 ++--
 net/core/dev.c                     |    4 ++--
 security/selinux/ss/services.c     |    2 +-
 12 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index a9ec910..e732530 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -972,7 +972,7 @@ static void disk_release(struct device *dev)
 {
 	struct gendisk *disk = dev_to_disk(dev);
 
-	kfree(disk->random);
+	_kfree(disk->random);
 	disk_replace_part_tbl(disk, NULL);
 	free_part_stats(&disk->part0);
 	kfree(disk);
diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index 5a76d05..e4517d7 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -160,7 +160,7 @@ static int dmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 static struct class dmi_class = {
 	.name = "dmi",
-	.dev_release = (void(*)(struct device *)) kfree,
+	.dev_release = (void(*)(struct device *)) _kfree,
 	.dev_uevent = dmi_dev_uevent,
 };
 
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index e42019e..0ca35e5 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -76,7 +76,7 @@ int intel_ddc_get_modes(struct intel_output *intel_output)
 		drm_mode_connector_update_edid_property(&intel_output->base,
 							edid);
 		ret = drm_add_edid_modes(&intel_output->base, edid);
-		kfree(edid);
+		_kfree(edid);
 	}
 
 	return ret;
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7fa3718..8809c9c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -526,6 +526,11 @@ struct sysinfo {
    aren't permitted). */
 #define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
 
+/* If the type of pointer x matches type, force a compilation error,
+   otherwise produce x as a result */
+#define PTR_OR_BB_ON_TYPE(x, type) (&x[sizeof(char[0 -			\
+			__builtin_types_compatible_p(typeof(x), type)])])
+
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 24c5602..7a17c3e 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -121,12 +121,15 @@ int kmem_ptr_validate(struct kmem_cache *cachep, const void *ptr);
 #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
 #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)
 
+/* This ensures that kfree is not called with a struct sk_buff* */
+#define kfree(x) _kfree(PTR_OR_BB_ON_TYPE((x), struct sk_buff*))
+
 /*
  * Common kmalloc functions provided by all allocators
  */
 void * __must_check __krealloc(const void *, size_t, gfp_t);
 void * __must_check krealloc(const void *, size_t, gfp_t);
-void kfree(const void *);
+void _kfree(const void *);
 void kzfree(const void *);
 size_t ksize(const void *);
 
diff --git a/kernel/exit.c b/kernel/exit.c
index efd30cc..96eb98a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1103,7 +1103,7 @@ NORET_TYPE void do_exit(long code)
 	if (unlikely(!list_empty(&tsk->pi_state_list)))
 		exit_pi_state_list(tsk);
 	if (unlikely(current->pi_state_cache))
-		kfree(current->pi_state_cache);
+		_kfree(current->pi_state_cache);
 #endif
 	/*
 	 * Make sure we are holding no locks:
diff --git a/lib/kref.c b/lib/kref.c
index 9ecd6e8..a6364df 100644
--- a/lib/kref.c
+++ b/lib/kref.c
@@ -62,7 +62,7 @@ void kref_get(struct kref *kref)
 int kref_put(struct kref *kref, void (*release)(struct kref *kref))
 {
 	WARN_ON(release == NULL);
-	WARN_ON(release == (void (*)(struct kref *))kfree);
+	WARN_ON(release == (void (*)(struct kref *))_kfree);
 
 	if (atomic_dec_and_test(&kref->refcount)) {
 		release(kref);
diff --git a/mm/slab.c b/mm/slab.c
index 4d00855..3acefac 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3711,7 +3711,7 @@ EXPORT_SYMBOL(kmem_cache_free);
  * Don't free memory not originally allocated by kmalloc()
  * or you will run into trouble.
  */
-void kfree(const void *objp)
+void _kfree(const void *objp)
 {
 	struct kmem_cache *c;
 	unsigned long flags;
@@ -3726,7 +3726,7 @@ void kfree(const void *objp)
 	__cache_free(c, (void *)objp);
 	local_irq_restore(flags);
 }
-EXPORT_SYMBOL(kfree);
+EXPORT_SYMBOL(_kfree);
 
 unsigned int kmem_cache_size(struct kmem_cache *cachep)
 {
diff --git a/mm/slob.c b/mm/slob.c
index 52bc8a2..1d8a3c4 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -487,7 +487,7 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 }
 EXPORT_SYMBOL(__kmalloc_node);
 
-void kfree(const void *block)
+void _kfree(const void *block)
 {
 	struct slob_page *sp;
 
@@ -502,7 +502,7 @@ void kfree(const void *block)
 	} else
 		put_page(&sp->page);
 }
-EXPORT_SYMBOL(kfree);
+EXPORT_SYMBOL(_kfree);
 
 /* can't use ksize for kmem_cache_alloc memory, only kmalloc */
 size_t ksize(const void *block)
diff --git a/mm/slub.c b/mm/slub.c
index 0280eee..f9fa9e1 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2738,7 +2738,7 @@ size_t ksize(const void *object)
 }
 EXPORT_SYMBOL(ksize);
 
-void kfree(const void *x)
+void _kfree(const void *x)
 {
 	struct page *page;
 	void *object = (void *)x;
@@ -2754,7 +2754,7 @@ void kfree(const void *x)
 	}
 	slab_free(page->slab, page, object, _RET_IP_);
 }
-EXPORT_SYMBOL(kfree);
+EXPORT_SYMBOL(_kfree);
 
 /*
  * kmem_cache_shrink removes empty slabs from the partial lists and sorts
diff --git a/net/core/dev.c b/net/core/dev.c
index f112970..74038ef 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2671,7 +2671,7 @@ void netif_napi_del(struct napi_struct *napi)
 	struct sk_buff *skb, *next;
 
 	list_del_init(&napi->dev_list);
-	kfree(napi->skb);
+	_kfree(napi->skb);
 
 	for (skb = napi->gro_list; skb; skb = next) {
 		next = skb->next;
@@ -4720,7 +4720,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 	if (!tx) {
 		printk(KERN_ERR "alloc_netdev: Unable to allocate "
 		       "tx qdiscs.\n");
-		kfree(p);
+		_kfree(p);
 		return NULL;
 	}
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index c65e4fe..cefe043 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2862,7 +2862,7 @@ static void security_netlbl_cache_add(struct netlbl_lsm_secattr *secattr,
 	}
 
 	*sid_cache = sid;
-	secattr->cache->free = kfree;
+	secattr->cache->free = _kfree;
 	secattr->cache->data = sid_cache;
 	secattr->flags |= NETLBL_SECATTR_CACHE;
 }

             reply	other threads:[~2009-03-13 16:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-13 16:17 Roel Kluin [this message]
2009-03-13 23:16 ` build bug on kfree(struct sk_buff*) Roel Kluin
2009-03-15 16:42   ` Roel Kluin
2009-03-15 19:33     ` Ben Hutchings

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=49BA8710.20808@gmail.com \
    --to=roel.kluin@gmail.com \
    --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.