diff mbox series

spl: ymodem: Fix buffer overflow during Image copy

Message ID 20220131041919.1679901-1-vigneshr@ti.com
State Accepted
Commit c1335e2ca5e3947a61d93c094fbb4a9be9afc4ff
Delegated to: Tom Rini
Headers show
Series spl: ymodem: Fix buffer overflow during Image copy | expand

Commit Message

Raghavendra, Vignesh Jan. 31, 2022, 4:19 a.m. UTC
ymodem_read_fit() driver will end copying up to BUF_SIZE boundary even
when requested size of copy operation is less than that.
For example, if offset = 0, size = 1440B, ymodem_read_fit() ends up
copying 2KB from offset = 0, to destination buffer addr

This causes data corruption when malloc'd buffer is passed during UART
boot since commit 03f1f78a9b44 ("spl: fit: Prefer a malloc()'d buffer
for loading images")

With this, UART boot works again on K3 (AM654, J7, AM64) family of
devices.

Fixes: 03f1f78a9b44 ("spl: fit: Prefer a malloc()'d buffer for loading images")
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 common/spl/spl_ymodem.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Tom Rini Feb. 4, 2022, 12:41 a.m. UTC | #1
On Mon, Jan 31, 2022 at 09:49:19AM +0530, Vignesh Raghavendra wrote:

> ymodem_read_fit() driver will end copying up to BUF_SIZE boundary even
> when requested size of copy operation is less than that.
> For example, if offset = 0, size = 1440B, ymodem_read_fit() ends up
> copying 2KB from offset = 0, to destination buffer addr
> 
> This causes data corruption when malloc'd buffer is passed during UART
> boot since commit 03f1f78a9b44 ("spl: fit: Prefer a malloc()'d buffer
> for loading images")
> 
> With this, UART boot works again on K3 (AM654, J7, AM64) family of
> devices.
> 
> Fixes: 03f1f78a9b44 ("spl: fit: Prefer a malloc()'d buffer for loading images")
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/common/spl/spl_ymodem.c b/common/spl/spl_ymodem.c
index 047df74856..fdd5261042 100644
--- a/common/spl/spl_ymodem.c
+++ b/common/spl/spl_ymodem.c
@@ -42,6 +42,7 @@  static ulong ymodem_read_fit(struct spl_load_info *load, ulong offset,
 	int res, err, buf_offset;
 	struct ymodem_fit_info *info = load->priv;
 	char *buf = info->buf;
+	ulong copy_size = size;
 
 	while (info->image_read < offset) {
 		res = xyzModem_stream_read(buf, BUF_SIZE, &err);
@@ -57,8 +58,14 @@  static ulong ymodem_read_fit(struct spl_load_info *load, ulong offset,
 			buf_offset = (info->image_read % BUF_SIZE);
 		else
 			buf_offset = BUF_SIZE;
+
+		if (res > copy_size) {
+			memcpy(addr, &buf[buf_offset - res], copy_size);
+			goto done;
+		}
 		memcpy(addr, &buf[buf_offset - res], res);
 		addr = addr + res;
+		copy_size -= res;
 	}
 
 	while (info->image_read < offset + size) {
@@ -66,11 +73,17 @@  static ulong ymodem_read_fit(struct spl_load_info *load, ulong offset,
 		if (res <= 0)
 			break;
 
-		memcpy(addr, buf, res);
 		info->image_read += res;
+		if (res > copy_size) {
+			memcpy(addr, buf, copy_size);
+			goto done;
+		}
+		memcpy(addr, buf, res);
 		addr += res;
+		copy_size -= res;
 	}
 
+done:
 	return size;
 }