Possible bug in USBHost_t36's memory allocation code

lukexyz

Active member
Cross-posting what I posted here: https://github.com/PaulStoffregen/USBHost_t36/issues/60

This code pattern is used a number of times in memory.cpp for memory allocation and recycling (for Device_t, Pipe_t, Transfer_t, etc.):

Code:
Device_t * USBHost::allocate_Device(void)
{
	Device_t *device = free_Device_list;
	if (device) free_Device_list = *(Device_t **)device;
	return device;
}

void USBHost::free_Device(Device_t *device)
{
	*(Device_t **)device = free_Device_list;
	free_Device_list = device;
}

I don't understand what the intent is here. If device has type Device_t *, then *(Device_t **)device can only make sense if the very first field in Device_t is of type Device_t *, i.e. the next field. But next is not the first field in Device_t. (Same for Pipe_t and Transfer_t.)

I think this usually does not cause a problem for Memory_t and Pipe_t, because there is only one of each allocated:

Code:
static Device_t memory_Device[1];
static Pipe_t memory_Pipe[1] __attribute__ ((aligned(32)));

However there are 4 Transfer_t structs allocated:

Code:
static Transfer_t memory_Transfer[4] __attribute__ ((aligned(32)));

therefore, these methods will corrupt memory, unless I'm misunderstanding the code:

Code:
Transfer_t * USBHostPort::allocate_Transfer(void)
{
	Transfer_t *transfer = free_Transfer_list;
	if (transfer) free_Transfer_list = *(Transfer_t **)transfer;
	return transfer;
}

void USBHostPort::free_Transfer(Transfer_t *transfer)
{
	*(Transfer_t **)transfer = free_Transfer_list;
	free_Transfer_list = transfer;
}
 
I guess the memory corruption doesn't matter, since the first field of the struct is only overwritten by a type-coerced pointer when the struct is added to the free list. But still, this is pretty ugly, especially when some of the affected structs already have next fields that could be used...
 
Memory allocators are allowed to break the rules as the objects are not "live" when they are in freelists (one hopes!)
In a full-blown garbage collector you normally have to completely subvert any type system anyway as memory is
recycled between objects of all types.
 
Back
Top