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);