Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 3 of 3

Thread: Possible bug in USBHost_t36's memory allocation code

  1. #1

    Possible bug in USBHost_t36's memory allocation code

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

  2. #2
    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...

  3. #3
    Senior Member
    Join Date
    Jul 2020
    Posts
    976
    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.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •