here's diff x/y. most of the big pieces in place, so now I want to
rearrange the deckchairs just the way I like. A bunch of unreleated
changes collected here instead of mailing out a pile of overlapping
diffs you could never apply independently. This settles things down in
prep for some more interesting changes.
- move poison prototypes from systm.h to malloc.h
- add poison_value() which returns the poison value appropriate for
this pointer
- use poison_value throughout malloc and pool
- increase poison size to 64. the old malloc code was effectively 128
(32 * sizeof(int)) and pool was whatever the item size is. i think 32
is a little on the small side, 64 should be enough to catch most errors.
- move kmem_freelist definition back into malloc, a forward decl
suffices.
- the magic field can remain in pool item headers unconditionally, so
remove the ifdef guard. i don't like size changing structs, even local
ones.
- make some POOL_DEBUG less conditional. this allows the pool_debug
sysctl to actually do something on release kernels. still won't have
much performance impact because all the poisoning is runtime guarded
as well.
- maybe some other stuff i forgot to mention. read the diff.
Index: sys/malloc.h
===================================================================
RCS file: /cvs/src/sys/sys/malloc.h,v
retrieving revision 1.103
diff -u -p -r1.103 malloc.h
--- sys/malloc.h 26 Mar 2013 16:36:01 -0000 1.103
+++ sys/malloc.h 31 Mar 2013 02:02:54 -0000
@@ -341,20 +341,7 @@ struct kmemusage {
#define ku_freecnt ku_un.freecnt
#define ku_pagecnt ku_un.pagecnt
-/*
- * Normally the freelist structure is used only to hold the list pointer
- * for free objects. However, when running with diagnostics, the first
- * 8 bytes of the structure is unused except for diagnostic information,
- * and the free list pointer is at offset 8 in the structure. Since the
- * first 8 bytes is the portion of the structure most often modified, this
- * helps to detect memory reuse problems and avoid free list corruption.
- */
-struct kmem_freelist {
- int32_t kf_spare0;
- int16_t kf_type;
- int16_t kf_spare1;
- SIMPLEQ_ENTRY(kmem_freelist) kf_flist;
-};
+struct kmem_freelist;
/*
* Set of buckets for each size of memory block that is retained
@@ -410,6 +397,10 @@ extern int sysctl_malloc(int *, u_int, v
size_t malloc_roundup(size_t);
void malloc_printit(int (*)(const char *, ...));
+
+void poison_mem(void *, size_t);
+int poison_check(void *, size_t, size_t *, int *);
+int32_t poison_value(void *);
#ifdef MALLOC_DEBUG
int debug_malloc(unsigned long, int, int, void **);
Index: sys/systm.h
===================================================================
RCS file: /cvs/src/sys/sys/systm.h,v
retrieving revision 1.96
diff -u -p -r1.96 systm.h
--- sys/systm.h 28 Mar 2013 16:41:39 -0000 1.96
+++ sys/systm.h 31 Mar 2013 02:01:21 -0000
@@ -187,9 +187,6 @@ extern int splassert_ctl;
void assertwaitok(void);
-void poison_mem(void *, size_t);
-int poison_check(void *, size_t, size_t *, int *);
-
void tablefull(const char *);
int kcopy(const void *, void *, size_t)
Index: kern/kern_malloc.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_malloc.c,v
retrieving revision 1.98
diff -u -p -r1.98 kern_malloc.c
--- kern/kern_malloc.c 28 Mar 2013 16:41:39 -0000 1.98
+++ kern/kern_malloc.c 31 Mar 2013 02:03:48 -0000
@@ -120,6 +120,21 @@ char *memall = NULL;
struct rwlock sysctl_kmemlock = RWLOCK_INITIALIZER("sysctlklk");
#endif
+/*
+ * Normally the freelist structure is used only to hold the list pointer
+ * for free objects. However, when running with diagnostics, the first
+ * 8 bytes of the structure is unused except for diagnostic information,
+ * and the free list pointer is at offset 8 in the structure. Since the
+ * first 8 bytes is the portion of the structure most often modified, this
+ * helps to detect memory reuse problems and avoid free list corruption.
+ */
+struct kmem_freelist {
+ int32_t kf_spare0;
+ int16_t kf_type;
+ int16_t kf_spare1;
+ SIMPLEQ_ENTRY(kmem_freelist) kf_flist;
+};
+
#ifdef DIAGNOSTIC
/*
* This structure provides a set of masks to catch unaligned frees.
@@ -131,16 +146,6 @@ const long addrmask[] = { 0,
0x00001fff, 0x00003fff, 0x00007fff, 0x0000ffff,
};
-/*
- * The FREELIST_MARKER is used as known text to copy into free objects so
- * that modifications after frees can be detected.
- */
-#ifdef DEADBEEF0
-#define FREELIST_MARKER ((unsigned) DEADBEEF0)
-#else
-#define FREELIST_MARKER ((unsigned) 0xdeadbeef)
-#endif
-
#endif /* DIAGNOSTIC */
#ifndef SMALL_KERNEL
@@ -413,7 +418,7 @@ free(void *addr, int type)
* Check for multiple frees. Use a quick check to see if
* it looks free before laboriously searching the freelist.
*/
- if (freep->kf_spare0 == FREELIST_MARKER) {
+ if (freep->kf_spare0 == poison_value(freep)) {
struct kmem_freelist *fp;
SIMPLEQ_FOREACH(fp, &kbp->kb_freelist, kf_flist) {
if (addr != fp)
@@ -429,7 +434,7 @@ free(void *addr, int type)
* when the object is reallocated.
*/
poison_mem(addr, size);
- freep->kf_spare0 = FREELIST_MARKER;
+ freep->kf_spare0 = poison_value(freep);
freep->kf_type = type;
#endif /* DIAGNOSTIC */
Index: kern/subr_poison.c
===================================================================
RCS file: /cvs/src/sys/kern/subr_poison.c,v
retrieving revision 1.1
diff -u -p -r1.1 subr_poison.c
--- kern/subr_poison.c 28 Mar 2013 16:41:39 -0000 1.1
+++ kern/subr_poison.c 31 Mar 2013 02:06:45 -0000
@@ -16,7 +16,7 @@
*/
#include <sys/types.h>
-#include <sys/systm.h>
+#include <sys/malloc.h>
/*
* The POISON is used as known text to copy into free objects so
@@ -27,33 +27,44 @@
#else
#define POISON ((unsigned) 0xdeadbeef)
#endif
-#define POISON_SIZE 32
+#define POISON_SIZE 64
+
+int32_t
+poison_value(void *v)
+{
+ return POISON;
+}
void
poison_mem(void *v, size_t len)
{
uint32_t *ip = v;
size_t i;
+ int32_t poison;
+
+ poison = poison_value(v);
if (len > POISON_SIZE)
len = POISON_SIZE;
len = len / sizeof(*ip);
for (i = 0; i < len; i++)
- ip[i] = POISON;
+ ip[i] = poison;
}
int
poison_check(void *v, size_t len, size_t *pidx, int *pval)
{
-
uint32_t *ip = v;
size_t i;
+ int32_t poison;
+
+ poison = poison_value(v);
if (len > POISON_SIZE)
len = POISON_SIZE;
len = len / sizeof(*ip);
for (i = 0; i < len; i++) {
- if (ip[i] != POISON) {
+ if (ip[i] != poison) {
*pidx = i;
*pval = ip[i];
return 1;
Index: kern/subr_pool.c
===================================================================
RCS file: /cvs/src/sys/kern/subr_pool.c,v
retrieving revision 1.116
diff -u -p -r1.116 subr_pool.c
--- kern/subr_pool.c 31 Mar 2013 00:03:26 -0000 1.116
+++ kern/subr_pool.c 31 Mar 2013 02:28:06 -0000
@@ -78,19 +78,11 @@ struct pool_item_header {
};
struct pool_item {
-#ifdef DIAGNOSTIC
u_int32_t pi_magic;
-#endif
/* Other entries use only this list entry */
SIMPLEQ_ENTRY(pool_item) pi_list;
};
-#ifdef DEADBEEF1
-#define PI_MAGIC DEADBEEF1
-#else
-#define PI_MAGIC 0xdeafbeef
-#endif
-
#ifdef POOL_DEBUG
int pool_debug = 1;
#else
@@ -463,7 +455,7 @@ pool_alloc_item_header(struct pool *pp,
ph = pool_get(&phpool, (flags & ~(PR_WAITOK | PR_ZERO)) |
PR_NOWAIT);
if (pool_debug && ph != NULL)
- ph->ph_magic = PI_MAGIC;
+ ph->ph_magic = poison_value(ph);
return (ph);
}
@@ -638,11 +630,10 @@ startover:
#endif
#ifdef DIAGNOSTIC
- if (pi->pi_magic != PI_MAGIC)
+ if (pi->pi_magic != poison_value(pi))
panic("pool_do_get(%s): free list modified: "
"page %p; item addr %p; offset 0x%x=0x%x",
pp->pr_wchan, ph->ph_page, pi, 0, pi->pi_magic);
-#ifdef POOL_DEBUG
if (pool_debug && ph->ph_magic) {
size_t pidx;
int pval;
@@ -655,7 +646,6 @@ startover:
pidx * sizeof(int), ip[pidx]);
}
}
-#endif /* POOL_DEBUG */
#endif /* DIAGNOSTIC */
/*
@@ -773,12 +763,10 @@ pool_do_put(struct pool *pp, void *v)
* Return to item list.
*/
#ifdef DIAGNOSTIC
- pi->pi_magic = PI_MAGIC;
-#ifdef POOL_DEBUG
+ pi->pi_magic = poison_value(pi);
if (ph->ph_magic) {
poison_mem(pi + 1, pp->pr_size - sizeof(*pi));
}
-#endif /* POOL_DEBUG */
#endif /* DIAGNOSTIC */
SIMPLEQ_INSERT_HEAD(&ph->ph_itemlist, pi, pi_list);
@@ -922,12 +910,10 @@ pool_prime_page(struct pool *pp, caddr_t
SIMPLEQ_INSERT_TAIL(&ph->ph_itemlist, pi, pi_list);
#ifdef DIAGNOSTIC
- pi->pi_magic = PI_MAGIC;
-#ifdef POOL_DEBUG
+ pi->pi_magic = poison_value(pi);
if (ph->ph_magic) {
poison_mem(pi + 1, pp->pr_size - sizeof(*pi));
}
-#endif /* POOL_DEBUG */
#endif /* DIAGNOSTIC */
cp = (caddr_t)(cp + pp->pr_size);
}
@@ -1143,7 +1129,7 @@ pool_print_pagelist(struct pool_pagelist
ph->ph_page, ph->ph_nmissing);
#ifdef DIAGNOSTIC
SIMPLEQ_FOREACH(pi, &ph->ph_itemlist, pi_list) {
- if (pi->pi_magic != PI_MAGIC) {
+ if (pi->pi_magic != poison_value(pi)) {
(*pr)("\t\t\titem %p, magic 0x%x\n",
pi, pi->pi_magic);
}
@@ -1298,7 +1284,7 @@ pool_chk_page(struct pool *pp, struct po
pi = SIMPLEQ_NEXT(pi,pi_list), n++) {
#ifdef DIAGNOSTIC
- if (pi->pi_magic != PI_MAGIC) {
+ if (pi->pi_magic != poison_value(pi)) {
printf("%s: ", label);
printf("pool(%s): free list modified: "
"page %p; item ordinal %d; addr %p "
@@ -1306,7 +1292,6 @@ pool_chk_page(struct pool *pp, struct po
pp->pr_wchan, ph->ph_page, n, pi, page,
0, pi->pi_magic);
}
-#ifdef POOL_DEBUG
if (pool_debug && ph->ph_magic) {
size_t pidx;
int pval;
@@ -1320,8 +1305,6 @@ pool_chk_page(struct pool *pp, struct po
page, pidx * sizeof(int), ip[pidx]);
}
}
-
-#endif /* POOL_DEBUG */
#endif /* DIAGNOSTIC */
page =
(caddr_t)((u_long)pi & pp->pr_alloc->pa_pagemask);
