Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32083 )
Change subject: libpayload: Fix potential NULL pointer dereference ......................................................................
libpayload: Fix potential NULL pointer dereference
Found-by: Klockwork BUG=NONE TEST=Boot to OS on GLK Sparky
Signed-off-by: Francois Toguo francois.toguo.fotso@intel.com Change-Id: I9d4636f0429de829e746909492c2f543026a02ac Reviewed-on: https://review.coreboot.org/c/coreboot/+/32083 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Lijian Zhao lijian.zhao@intel.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/drivers/usb/ehci.c M payloads/libpayload/drivers/usb/ohci.c M payloads/libpayload/drivers/usb/uhci.c 3 files changed, 44 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Lijian Zhao: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/usb/ehci.c b/payloads/libpayload/drivers/usb/ehci.c index 30cf191..3d6c077 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 DMA memory to create #reqcount TD.\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,8 @@ /* create one more qTD */ intr_qtd_t *const next_td = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!next_td) + fatal("Not enough DMA memory to create TD.\n"); cur_td->td.next_qtd = virt_to_phys(&next_td->td); cur_td->next = next_td; cur_td = next_td; @@ -651,6 +655,8 @@
/* create spare qTD */ intrq->spare = (intr_qtd_t *)dma_memalign(64, sizeof(intr_qtd_t)); + if (!intrq->spare) + fatal("Not enough DMA memory to create spare qTD.\n"); intrq->spare->data = data;
/* initialize QH */ @@ -824,14 +830,17 @@ * and doesn't violate the standard. */ EHCI_INST(controller)->dummy_qh = (ehci_qh_t *)dma_memalign(64, sizeof(ehci_qh_t)); + if (!EHCI_INST(controller)->dummy_qh) + fatal("Not enough DMA memory for EHCI dummy TD.\n"); 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; + periodic_list[i] = + virt_to_phys(EHCI_INST(controller)->dummy_qh) + | PS_TYPE_QH;
/* Make sure periodic schedule is disabled */ ehci_set_periodic_schedule(EHCI_INST(controller), 0); diff --git a/payloads/libpayload/drivers/usb/ohci.c b/payloads/libpayload/drivers/usb/ohci.c index 1964689..ecd1084 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,16 @@
/* Final dummy TD. */ td_t *const final_td = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!final_td) + fatal("Not enough DMA memory for OHCI dummy TD!\n"); memset((void *)final_td, 0, sizeof(*final_td)); /* Linked to the previous. */ cur->next_td = virt_to_phys(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 +533,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 bulk transfer.\n"); memset((void *)first_td, 0, sizeof(*first_td)); cur = next = first_td;
@@ -557,6 +573,8 @@ } /* One more TD. */ next = (td_t *)dma_memalign(sizeof(td_t), sizeof(td_t)); + if (!next) + fatal("Not enough DMA mem for TD bulk transfer.\n"); memset((void *)next, 0, sizeof(*next)); /* Linked to the previous. */ cur->next_td = virt_to_phys(next); @@ -569,6 +587,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 +685,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(intrq); + return NULL; + } memset(intrq, 0, sizeof(*intrq)); intrq->data = (u8 *)dma_malloc(reqcount * reqsize); intrq->reqsize = reqsize; @@ -674,6 +699,8 @@ 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) + fatal("Not enough DMA mem to transfer descriptor.\n"); ++intrq->remaining_tds; ohci_fill_intrq_td(td, intrq, cur_data); cur_data += reqsize; @@ -686,6 +713,8 @@
/* Create last, dummy TD. */ intrq_td_t *dummy_td = dma_memalign(sizeof(dummy_td->td), sizeof(*dummy_td)); + if (!dummy_td) + fatal("Not enough memory to add dummy TD.\n"); memset(dummy_td, 0, sizeof(*dummy_td)); dummy_td->intrq = intrq; if (last_td) 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++) {