[U-Boot] [PATCH 1/5] mmc: dw_mmc: prevent silent memory corruption when stack and heap are too small

Alberto Panizzo alberto at amarulasolutions.com
Wed Jul 4 19:41:23 UTC 2018


ALLOC_CACHE_ALIGN_BUFFER was called here in a way to alloc in stack a
possible huge quantity of memory depending on data transer size.

Es: loading kernel 8MB from eMMC we have
Transfer size:   0x800000
Block size:      0x200
Transfer blocks: 0x4000
struct size:     0x10
Stack allocation: ((0x200 / 8) + 1) * 0x10 = 0x8010 (~32KB)

Since this allocation is done on stack, there is no current way to get
an error on stack memory limit exceeded, overlapping heap space on
environments with very strict stack + heap limits like TPL or SPL (where
malloc size can be 16KB).
Results are silent corruptions of heap on mmc transfer and random errors
or CPU hang.

Using malloc_cache_aligned() we will alloc slightly bigger buffers
but we do have evidence about memory allocation failure allowing developer
to recognize the issue and take actions.

Signed-off-by: Alberto Panizzo <alberto at amarulasolutions.com>
---
 drivers/mmc/dw_mmc.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 13180fc..0126563 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -194,8 +194,9 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 {
 #endif
 	struct dwmci_host *host = mmc->priv;
-	ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
-				 data ? DIV_ROUND_UP(data->blocks, 8) : 0);
+	struct dwmci_idmac *cur_idmac =
+		malloc_cache_aligned(sizeof(struct dwmci_idmac) *
+			(1 + (data ? DIV_ROUND_UP(data->blocks, 8) : 0)));
 	int ret = 0, flags = 0, i;
 	unsigned int timeout = 500;
 	u32 retry = 100000;
@@ -203,10 +204,18 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	ulong start = get_timer(0);
 	struct bounce_buffer bbstate;
 
+	if (!cur_idmac) {
+		debug("%s: Cannot allocate 0x%x bytes\n", __func__,
+		      sizeof(struct dwmci_idmac) *
+		      (1 + (data ? DIV_ROUND_UP(data->blocks, 8) : 0)));
+		return -ENOMEM;
+	}
+
 	while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) {
 		if (get_timer(start) > timeout) {
 			debug("%s: Timeout on data busy\n", __func__);
-			return -ETIMEDOUT;
+			ret = -ETIMEDOUT;
+			goto free_idmac;
 		}
 	}
 
@@ -238,8 +247,10 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	if (data)
 		flags = dwmci_set_transfer_mode(host, data);
 
-	if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type & MMC_RSP_BUSY))
-		return -1;
+	if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type & MMC_RSP_BUSY)) {
+		ret = -EIO;
+		goto free_idmac;
+	}
 
 	if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
 		flags |= DWMCI_CMD_ABORT_STOP;
@@ -272,7 +283,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	if (i == retry) {
 		debug("%s: Timeout.\n", __func__);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto free_idmac;
 	}
 
 	if (mask & DWMCI_INTMSK_RTO) {
@@ -285,10 +297,12 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 		 * CMD8, please keep that in mind.
 		 */
 		debug("%s: Response Timeout.\n", __func__);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto free_idmac;
 	} else if (mask & DWMCI_INTMSK_RE) {
 		debug("%s: Response Error.\n", __func__);
-		return -EIO;
+		ret = -EIO;
+		goto free_idmac;
 	}
 
 
@@ -317,6 +331,9 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	udelay(100);
 
+free_idmac:
+	free(cur_idmac);
+
 	return ret;
 }
 
-- 
2.7.4



More information about the U-Boot mailing list