New subject: Change in ...coreboot[master]: payload/libpayload: This is a fix for potential NULL pointer derefere...

27 Mar 2019

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++) {
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/32083
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9d4636f0429de829e746909492c2f543026a02ac
Gerrit-Change-Number: 32083
Gerrit-PatchSet: 1
Gerrit-Owner: Francois Toguo Fotso francois.toguo.fotso@intel.com
Gerrit-MessageType: newchange



    

Change in ...coreboot[master]: payload/libpayload: This is a fixe for potential NULL pointer derefer...