Modify

Opened 6 years ago

Closed 5 years ago

#146 closed defect (fixed)

memalign requests huge amounts of memory for large alignments

Reported by: wangqingpei@… Owned by: oxygene
Priority: blocker Milestone:
Component: libpayload Keywords:
Cc: coreboot@…, stepan@…, Dependencies:
Patch Status: there is no patch

Description

with libpayload/libc/malloc.c memalign, it crash if tring to memalign with 4kb about 4k boundary

Attachments (1)

20100118-1-memalign.diff (478 bytes) - added by oxygene 6 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 6 years ago by wangqingpei@…

in libpayload/drivers/usb/uhci.c line 127 the function uhci_init is used for initializing the UHCI controller.

hci_t *
uhci_init (pcidev_t addr)
{
        int i;
        hci_t *controller = new_controller ();
        printf("the  malloc 1\n");
        controller->instance = malloc (sizeof (uhci_t));
        controller->start = uhci_start;
        controller->stop = uhci_stop;
        controller->reset = uhci_reset;
        controller->shutdown = uhci_shutdown;
        controller->packet = uhci_packet;
        controller->bulk = uhci_bulk;
        controller->control = uhci_control;
        controller->create_intr_queue = uhci_create_intr_queue;
        controller->destroy_intr_queue = uhci_destroy_intr_queue;
        controller->poll_intr_queue = uhci_poll_intr_queue;
        for (i = 0; i < 128; i++) {
                controller->devices[i] = 0;
        }
        init_device_entry (controller, 0);
        UHCI_INST (controller)->roothub = controller->devices[0];

        controller->bus_address = addr;
        controller->reg_base = pci_read_config32 (controller->bus_address, 0x20) & ~1;  /* ~1 clears the register type indicator that is set to 1 for IO space */

        /* kill legacy support handler */
        uhci_stop (controller);
        mdelay (1);
        uhci_reg_write16 (controller, USBSTS, 0x3f);
        pci_write_config32 (controller->bus_address, 0xc0, 0x8f00);
        printf("the memalign 2\n");
        UHCI_INST (controller)->framelistptr = memalign (0x1000, 1024 * sizeof (flistp_t *));   /* 4kb aligned to 4kb */
/* in this line which use mamalign to malloc 4Kb with 4k boundary, i tracked with memalign, find it failed at 
align_regions = allocate_region(align_regions, align, (size/align<99)?100:((size/align)+1));
*/

        memset (UHCI_INST (controller)->framelistptr, 0,
                1024 * sizeof (flistp_t));
        printf("the step 3 passed\n");

if i narrow the boundary from 0x1000 to 0x10, then the crash disappeared. what's why i though the function memalign has an bug, in my view, even the memory allocate failed, it should not crashed. but instead , just return with error

Changed 6 years ago by oxygene

comment:2 Changed 6 years ago by oxygene

  • Owner changed from somebody to oxygene
  • Status changed from new to assigned

Attached patch changes the behaviour of memalign, which should help in your case. It's also posted to the mailing list, see http://www.coreboot.org/pipermail/coreboot/2010-January/055269.html

comment:3 Changed 5 years ago by oxygene

  • Keywords memalign crashed removed
  • Resolution set to fixed
  • Status changed from assigned to closed
  • Summary changed from memalign faild with 4k boundary to memalign requests huge amounts of memory for large alignments

Fixed in r5298, which included the attached patch.

memalign doesn't try to allocate at least 100 units of memory (which meant 400kb for 4kb of 4kb-aligned memory. It only allocates at least a kilobyte of data, which should match most use cases: For more common 16byte requests, 1kb is allocated and pooled for 16byte requests, for relatively rare 4kb requests, 4kb is allocated (as it's already >1kb)

Add Comment

Modify Ticket

Action
as closed The owner will remain oxygene.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.