Francois Toguo Fotso has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32083
Change subject: payload/libpayload: This is a fixe for potential NULL pointer dereferences ......................................................................
payload/libpayload: This is a fixe for potential NULL pointer dereferences
These potential issues are flagged in Klockwork reports
BUG=NONE TEST=Boot to OS on GLK Sparky
Signed-off-by: Francois Toguo francois.toguo.fotso@intel.com Change-Id: I9d4636f0429de829e746909492c2f543026a02ac --- M payloads/libpayload/drivers/usb/ehci.c M payloads/libpayload/drivers/usb/ohci.c M payloads/libpayload/drivers/usb/uhci.c 3 files changed, 76 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/32083/1
diff --git a/payloads/libpayload/drivers/usb/ehci.c b/payloads/libpayload/drivers/usb/ehci.c index 30cf191..61a39a7 100644 --- a/payloads/libpayload/drivers/usb/ehci.c +++ b/payloads/libpayload/drivers/usb/ehci.c @@ -634,6 +634,8 @@
/* create #reqcount transfer descriptors (qTDs) */ intrq->head = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!intrq->head) + fatal("Not enough memory to create #reqcount transfer descriptors. \n"); intr_qtd_t *cur_td = intrq->head; for (i = 0; i < reqcount; ++i) { fill_intr_queue_td(intrq, cur_td, data); @@ -642,6 +644,11 @@ /* create one more qTD */ intr_qtd_t *const next_td = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!next_td) { + usb_debug("Not enough memory to add transfer descriptor. \n"); + free(next_td); + continue; + } cur_td->td.next_qtd = virt_to_phys(&next_td->td); cur_td->next = next_td; cur_td = next_td; @@ -651,6 +658,8 @@
/* create spare qTD */ intrq->spare = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!intrq->spare) + fatal("Not enough memory to create spare transfer descriptors.\n"); intrq->spare->data = data;
/* initialize QH */ @@ -824,15 +833,20 @@ * and doesn't violate the standard. */ EHCI_INST(controller)->dummy_qh = (ehci_qh_t *)dma_memalign(64, sizeof(ehci_qh_t)); - memset((void *)EHCI_INST(controller)->dummy_qh, 0, - sizeof(*EHCI_INST(controller)->dummy_qh)); - EHCI_INST(controller)->dummy_qh->horiz_link_ptr = QH_TERMINATE; - EHCI_INST(controller)->dummy_qh->td.next_qtd = QH_TERMINATE; - EHCI_INST(controller)->dummy_qh->td.alt_next_qtd = QH_TERMINATE; - for (i = 0; i < 1024; ++i) - periodic_list[i] = virt_to_phys(EHCI_INST(controller)->dummy_qh) - | PS_TYPE_QH; - + if (EHCI_INST(controller)->dummy_qh) { + memset((void *)EHCI_INST(controller)->dummy_qh, 0, + sizeof(*EHCI_INST(controller)->dummy_qh)); + EHCI_INST(controller)->dummy_qh->horiz_link_ptr = QH_TERMINATE; + EHCI_INST(controller)->dummy_qh->td.next_qtd = QH_TERMINATE; + EHCI_INST(controller)->dummy_qh->td.alt_next_qtd = QH_TERMINATE; + for (i = 0; i < 1024; ++i) + periodic_list[i] = virt_to_phys(EHCI_INST(controller)->dummy_qh) + | PS_TYPE_QH; + } else { + usb_debug("Not enough DMA memory for EHCI dummy QH!\n"); + free(EHCI_INST(controller)->dummy_qh); + } + /* Make sure periodic schedule is disabled */ ehci_set_periodic_schedule(EHCI_INST(controller), 0); /* Set periodic frame list pointer */ diff --git a/payloads/libpayload/drivers/usb/ohci.c b/payloads/libpayload/drivers/usb/ohci.c index 1964689..045bb7a 100644 --- a/payloads/libpayload/drivers/usb/ohci.c +++ b/payloads/libpayload/drivers/usb/ohci.c @@ -212,6 +212,8 @@ udelay (10); /* at most 10us for reset to complete. State must be set to Operational within 2ms (5.1.1.4) */ OHCI_INST (controller)->opreg->HcFmInterval = interval; OHCI_INST (controller)->hcca = dma_memalign(256, 256); + if (!OHCI_INST(controller)->hcca) + fatal("Not enough DMA memory for OHCI hcca.\n"); memset((void*)OHCI_INST (controller)->hcca, 0, 256);
if (dma_initialized()) { @@ -223,6 +225,8 @@ /* Initialize interrupt table. */ u32 *const intr_table = OHCI_INST(controller)->hcca->HccaInterruptTable; ed_t *const periodic_ed = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!periodic_ed) + fatal("Not enough DMA memory for OHCI interrupt table .\n"); memset((void *)periodic_ed, 0, sizeof(*periodic_ed)); for (i = 0; i < 32; ++i) intr_table[i] = virt_to_phys(periodic_ed); @@ -378,6 +382,8 @@
/* First TD. */ td_t *const first_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!first_td) + fatal("Not enough DMA memory for OHCI first TD in buffer.\n"); memset((void *)first_td, 0, sizeof(*first_td)); cur = first_td;
@@ -392,6 +398,8 @@ while (pages > 0) { /* One more TD. */ td_t *const next = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next) + fatal("Not enough DMA memory for OHCI new page.\n"); memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next); @@ -426,6 +434,8 @@
/* One more TD. */ td_t *const next_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next_td) + fatal("Not enough DMA memory for OHCI additional TD.\n"); memset((void *)next_td, 0, sizeof(*next_td)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next_td); @@ -441,12 +451,18 @@
/* Final dummy TD. */ td_t *const final_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); - memset((void *)final_td, 0, sizeof(*final_td)); - /* Linked to the previous. */ - cur->next_td = virt_to_phys(final_td); - + if (final_td) { + memset((void *)final_td, 0, sizeof(*final_td)); + /* Linked to the previous. */ + cur->next_td = virt_to_phys(final_td); + } else { + usb_debug("Not enough DMA memory for OHCI dummy TD!\n"); + free(final_td); + } /* Data structures */ ed_t *head = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!head) + fatal("Not enough DMA memory for OHCI data structures.\n"); memset((void*)head, 0, sizeof(*head)); head->config = (dev->address << ED_FUNC_SHIFT) | (0 << ED_EP_SHIFT) | @@ -519,6 +535,8 @@
/* First TD. */ td_t *const first_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!first_td) + fatal("Not enough DMA memory for OHCI first TD of bulk transfer.\n"); memset((void *)first_td, 0, sizeof(*first_td)); cur = next = first_td;
@@ -557,6 +575,10 @@ } /* One more TD. */ next = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next) { + usb_debug("Not enough DMA memory for OHCI addition TD in bulk transfer.\n"); + break; + } memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next); @@ -569,6 +591,8 @@
/* Data structures */ ed_t *head = dma_memalign(sizeof(ed_t), sizeof(ed_t)); + if (!head) + fatal("Not enough DMA memory for OHCI bulk transfer's head.\n"); memset((void*)head, 0, sizeof(*head)); head->config = (ep->dev->address << ED_FUNC_SHIFT) | ((ep->endpoint & 0xf) << ED_EP_SHIFT) | @@ -665,6 +689,11 @@
intr_queue_t *const intrq = (intr_queue_t *)dma_memalign(sizeof(intrq->ed), sizeof(*intrq)); + if (!intrq) { + usb_debug("Not enough DMA memory for intr queue \n"); + free(final_td); + return NULL; + } memset(intrq, 0, sizeof(*intrq)); intrq->data = (u8 *)dma_malloc(reqcount * reqsize); intrq->reqsize = reqsize; @@ -674,6 +703,11 @@ u8 *cur_data = intrq->data; for (i = 0; i < reqcount; ++i) { intrq_td_t *const td = dma_memalign(sizeof(td->td), sizeof(*td)); + if (!td) { + usb_debug("Not enough memory to add transfer descriptor. \n"); + free(td); + continue; + } ++intrq->remaining_tds; ohci_fill_intrq_td(td, intrq, cur_data); cur_data += reqsize; @@ -686,12 +720,17 @@
/* Create last, dummy TD. */ intrq_td_t *dummy_td = dma_memalign(sizeof(dummy_td->td), sizeof(*dummy_td)); - memset(dummy_td, 0, sizeof(*dummy_td)); - dummy_td->intrq = intrq; - if (last_td) - last_td->td.next_td = virt_to_phys(&dummy_td->td); - last_td = dummy_td; - + if (dummy_td) { + memset(dummy_td, 0, sizeof(*dummy_td)); + dummy_td->intrq = intrq; + if (last_td) + last_td->td.next_td = virt_to_phys(&dummy_td->td); + last_td = dummy_td; + } else { + usb_debug("Not enough memory to add dummy TD. \n"); + free(dummy_td); + } + /* Initialize ED. */ intrq->ed.config = (ep->dev->address << ED_FUNC_SHIFT) | ((ep->endpoint & 0xf) << ED_EP_SHIFT) | diff --git a/payloads/libpayload/drivers/usb/uhci.c b/payloads/libpayload/drivers/usb/uhci.c index 70071eb..e62fd25 100644 --- a/payloads/libpayload/drivers/usb/uhci.c +++ b/payloads/libpayload/drivers/usb/uhci.c @@ -317,6 +317,8 @@ unsigned short req = ((unsigned short *) devreq)[0]; int i; td_t *tds = memalign (16, sizeof (td_t) * count); + if (!tds) + fatal("Not enough memory for uhci control.\n"); memset (tds, 0, sizeof (td_t) * count); count--; /* to compensate for 0-indexed array */ for (i = 0; i < count; i++) { @@ -386,6 +388,8 @@ if (numpackets == 0) return 0; td_t *tds = memalign (16, sizeof (td_t) * numpackets); + if (!tds) + fatal("Not enough memory for packets scheduling.\n"); memset (tds, 0, sizeof (td_t) * numpackets); int i; for (i = 0; i < numpackets; i++) {