d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix critical stability problems that can cause UMA mbuf cluster

Browse Source 
state management corruption, mbuf leaks, general mbuf corruption,
and at least on i386 a first level splash damage radius that
encompasses up to about half a megabyte of the memory after
an mbuf cluster's allocation slab.  In short, this has caused
instability nightmares anywhere the right kind of network traffic
is present.

When the polymorphic refcount slabs were added to UMA, the new types
were not used pervasively.  In particular, the slab management
structure was turned into one for refcounts, and one for non-refcounts
(supposed to be mostly like the old slab management structure),
but the latter was almost always used through out.  In general, every
access to zones with UMA_ZONE_REFCNT turned on corrupted the
"next free" slab offset offset and the refcount with each other and
with other allocations (on i386, 2 mbuf clusters per 4096 byte slab).

Fix things so that the right type is used to access refcounted zones
where it was not before.  There are additional errors in gross
overestimation of padding, it seems, that would cause a large kegs
(nee zones) to be allocated when small ones would do.  Unless I have
analyzed this incorrectly, it is not directly harmful.
This commit is contained in:
Brian Feldman 2004-10-08 20:19:29 +00:00
parent 182cebd7ec
commit ab14a3f7aa
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=136276
2 changed files with 64 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
sys/vm/uma_core.c
View File

@ -232,7 +232,7 @@ void uma_print_stats(void);
static int sysctl_vm_zone(SYSCTL_HANDLER_ARGS);
#ifdef WITNESS
static int nosleepwithlocks = 1;
static int nosleepwithlocks = 0;
SYSCTL_INT(_debug, OID_AUTO, nosleepwithlocks, CTLFLAG_RW, &nosleepwithlocks,
0, "Convert M_WAITOK to M_NOWAIT to avoid lock-held-across-sleep paths");
#else
@ -825,13 +825,16 @@ slab_zalloc(uma_zone_t zone, int wait)
slab->us_freecount = keg->uk_ipers;
slab->us_firstfree = 0;
slab->us_flags = flags;
for (i = 0; i < keg->uk_ipers; i++)
slab->us_freelist[i].us_item = i+1;
if (keg->uk_flags & UMA_ZONE_REFCNT) {
slabref = (uma_slabrefcnt_t)slab;
for (i = 0; i < keg->uk_ipers; i++)
for (i = 0; i < keg->uk_ipers; i++) {
slabref->us_freelist[i].us_refcnt = 0;
slabref->us_freelist[i].us_item = i+1;
}
} else {
for (i = 0; i < keg->uk_ipers; i++)
slab->us_freelist[i].us_item = i+1;
}
if (keg->uk_init != NULL) {
@ -1983,13 +1986,19 @@ static void *
uma_slab_alloc(uma_zone_t zone, uma_slab_t slab)
{
uma_keg_t keg;
uma_slabrefcnt_t slabref;
void *item;
u_int8_t freei;
keg = zone->uz_keg;
freei = slab->us_firstfree;
slab->us_firstfree = slab->us_freelist[freei].us_item;
if (keg->uk_flags & UMA_ZONE_REFCNT) {
slabref = (uma_slabrefcnt_t)slab;
slab->us_firstfree = slabref->us_freelist[freei].us_item;
} else {
slab->us_firstfree = slab->us_freelist[freei].us_item;
}
item = slab->us_data + (keg->uk_rsize * freei);
slab->us_freecount--;
@ -2339,6 +2348,7 @@ uma_zfree_internal(uma_zone_t zone, void *item, void *udata,
enum zfreeskip skip)
{
uma_slab_t slab;
uma_slabrefcnt_t slabref;
uma_keg_t keg;
u_int8_t *mem;
u_int8_t freei;
@ -2382,7 +2392,12 @@ uma_zfree_internal(uma_zone_t zone, void *item, void *udata,
uma_dbg_free(zone, slab, item);
#endif
slab->us_freelist[freei].us_item = slab->us_firstfree;
if (keg->uk_flags & UMA_ZONE_REFCNT) {
slabref = (uma_slabrefcnt_t)slab;
slabref->us_freelist[freei].us_item = slab->us_firstfree;
} else {
slab->us_freelist[freei].us_item = slab->us_firstfree;
}
slab->us_firstfree = freei;
slab->us_freecount++;
@ -2545,18 +2560,19 @@ uma_prealloc(uma_zone_t zone, int items)
u_int32_t *
uma_find_refcnt(uma_zone_t zone, void *item)
{
uma_slabrefcnt_t slab;
uma_slabrefcnt_t slabref;
uma_keg_t keg;
u_int32_t *refcnt;
int idx;
keg = zone->uz_keg;
slab = (uma_slabrefcnt_t)vtoslab((vm_offset_t)item & (~UMA_SLAB_MASK));
KASSERT(slab != NULL,
slabref = (uma_slabrefcnt_t)vtoslab((vm_offset_t)item &
(~UMA_SLAB_MASK));
KASSERT(slabref != NULL && slabref->us_keg->uk_flags & UMA_ZONE_REFCNT,
("uma_find_refcnt(): zone possibly not UMA_ZONE_REFCNT"));
idx = ((unsigned long)item - (unsigned long)slab->us_data)
idx = ((unsigned long)item - (unsigned long)slabref->us_data)
/ keg->uk_rsize;
refcnt = &(slab->us_freelist[idx].us_refcnt);
refcnt = &slabref->us_freelist[idx].us_refcnt;
return refcnt;
}

50
sys/vm/uma_dbg.c
View File

@ -218,6 +218,7 @@ void
uma_dbg_alloc(uma_zone_t zone, uma_slab_t slab, void *item)
{
uma_keg_t keg;
uma_slabrefcnt_t slabref;
int freei;
keg = zone->uz_keg;
@ -231,7 +232,12 @@ uma_dbg_alloc(uma_zone_t zone, uma_slab_t slab, void *item)
freei = ((unsigned long)item - (unsigned long)slab->us_data)
/ keg->uk_rsize;
slab->us_freelist[freei].us_item = 255;
if (keg->uk_flags & UMA_ZONE_REFCNT) {
slabref = (uma_slabrefcnt_t)slab;
slabref->us_freelist[freei].us_item = 255;
} else {
slab->us_freelist[freei].us_item = 255;
}
return;
}
@ -246,6 +252,7 @@ void
uma_dbg_free(uma_zone_t zone, uma_slab_t slab, void *item)
{
uma_keg_t keg;
uma_slabrefcnt_t slabref;
int freei;
keg = zone->uz_keg;
@ -270,17 +277,34 @@ uma_dbg_free(uma_zone_t zone, uma_slab_t slab, void *item)
(freei * keg->uk_rsize) + slab->us_data);
}
if (slab->us_freelist[freei].us_item != 255) {
printf("Slab at %p, freei %d = %d.\n",
slab, freei, slab->us_freelist[freei].us_item);
panic("Duplicate free of item %p from zone %p(%s)\n",
item, zone, zone->uz_name);
}
if (keg->uk_flags & UMA_ZONE_REFCNT) {
slabref = (uma_slabrefcnt_t)slab;
if (slabref->us_freelist[freei].us_item != 255) {
printf("Slab at %p, freei %d = %d.\n",
slab, freei, slabref->us_freelist[freei].us_item);
panic("Duplicate free of item %p from zone %p(%s)\n",
item, zone, zone->uz_name);
}
/*
* When this is actually linked into the slab this will change.
* Until then the count of valid slabs will make sure we don't
* accidentally follow this and assume it's a valid index.
*/
slab->us_freelist[freei].us_item = 0;
/*
* When this is actually linked into the slab this will change.
* Until then the count of valid slabs will make sure we don't
* accidentally follow this and assume it's a valid index.
*/
slabref->us_freelist[freei].us_item = 0;
} else {
if (slab->us_freelist[freei].us_item != 255) {
printf("Slab at %p, freei %d = %d.\n",
slab, freei, slab->us_freelist[freei].us_item);
panic("Duplicate free of item %p from zone %p(%s)\n",
item, zone, zone->uz_name);
}
/*
* When this is actually linked into the slab this will change.
* Until then the count of valid slabs will make sure we don't
* accidentally follow this and assume it's a valid index.
*/
slab->us_freelist[freei].us_item = 0;
}
}