From 1e7478461bb4e8842f1ca8e5ffb5a441041b0753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:38 +0200 Subject: xyz-modem: Fix crash after cancelling transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variable xyz.len is set to -1 on error. At the end xyzModem_stream_read() function calls memcpy() with length from variable xyz.len. If this variable is set to -1 then value passed to memcpy is casted to unsigned value, which means to copy whole address space. Which then cause U-Boot crash. E.g. on arm64 it cause CPU crash: "Synchronous Abort" handler, esr 0x96000006 Fix this issue by checking that value stored in xyz.len is valid prior trying to use it. Signed-off-by: Pali Rohár Acked-by: Heinrich Schuchardt --- common/xyzModem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/xyzModem.c b/common/xyzModem.c index fc3459e..b1b72aa 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -494,7 +494,7 @@ xyzModem_stream_read (char *buf, int size, int *err) total = 0; stat = xyzModem_cancel; /* Try and get 'size' bytes into the buffer */ - while (!xyz.at_eof && (size > 0)) + while (!xyz.at_eof && xyz.len >= 0 && (size > 0)) { if (xyz.len == 0) { @@ -587,7 +587,7 @@ xyzModem_stream_read (char *buf, int size, int *err) } } /* Don't "read" data from the EOF protocol package */ - if (!xyz.at_eof) + if (!xyz.at_eof && xyz.len > 0) { len = xyz.len; if (size < len) -- cgit v1.1 From 15c27a5a223717041d4acba8a07db846df674073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:39 +0200 Subject: xyz-modem: Fix x-modem "xyzModem_eof error" at the end of file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In x-modem protocol EOF is not an error state at the end of file. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- common/xyzModem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/xyzModem.c b/common/xyzModem.c index b1b72aa..631c44e 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -572,6 +572,8 @@ xyzModem_stream_read (char *buf, int size, int *err) CYGACC_COMM_IF_PUTC (*xyz.__chan, ACK); ZM_DEBUG (zm_dprintf ("FINAL ACK (%d)\n", __LINE__)); } + else + stat = 0; xyz.at_eof = true; break; } -- cgit v1.1 From f05d69bd0a264d622efef9649187946cfbccb930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:40 +0200 Subject: xyz-modem: Put xyzModem_stream_close debug diagnostic message into ZM_DEBUG() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is how all other debug / diagnostic messages are handled. Signed-off-by: Pali Rohár --- common/xyzModem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/xyzModem.c b/common/xyzModem.c index 631c44e..c200c9f 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -608,10 +608,10 @@ xyzModem_stream_read (char *buf, int size, int *err) void xyzModem_stream_close (int *err) { - diag_printf + ZM_DEBUG (zm_dprintf ("xyzModem - %s mode, %d(SOH)/%d(STX)/%d(CAN) packets, %d retries\n", xyz.crc_mode ? "CRC" : "Cksum", xyz.total_SOH, xyz.total_STX, - xyz.total_CAN, xyz.total_retries); + xyz.total_CAN, xyz.total_retries)); ZM_DEBUG (zm_flush ()); } -- cgit v1.1 From 1f26c49ea1d968ff59abe804af14314d5f5dc8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:41 +0200 Subject: xyz-modem: Close stream after processing/sending terminate sequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Obviously it is not possible to send terminate sequence over stream after closing stream. Signed-off-by: Pali Rohár --- cmd/load.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/load.c b/cmd/load.c index 3904e13..e0c896b 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -1009,8 +1009,8 @@ static ulong load_serial_ymodem(ulong offset, int mode) printf("%s\n", xyzModem_error(err)); } - xyzModem_stream_close(&err); xyzModem_stream_terminate(false, &getcxmodem); + xyzModem_stream_close(&err); flush_cache(offset, ALIGN(size, ARCH_DMA_MINALIGN)); -- cgit v1.1 From 081bd249d119ecad114fd2560974688d8a5ce059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:42 +0200 Subject: xyz-modem: Properly abort/terminate transfer on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transfer termination tries to instruct sender that transfer was terminated. Print error message and indicates aborted transfer in return value. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- cmd/load.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/cmd/load.c b/cmd/load.c index e0c896b..549e563 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -978,6 +978,7 @@ static ulong load_serial_ymodem(ulong offset, int mode) res = xyzModem_stream_open(&info, &err); if (!res) { + err = 0; while ((res = xyzModem_stream_read(ymodemBuf, 1024, &err)) > 0) { store_addr = addr + offset; @@ -990,6 +991,9 @@ static ulong load_serial_ymodem(ulong offset, int mode) rc = flash_write((char *) ymodemBuf, store_addr, res); if (rc != 0) { + xyzModem_stream_terminate(true, &getcxmodem); + xyzModem_stream_close(&err); + printf("\n"); flash_perror(rc); return (~0); } @@ -1001,12 +1005,20 @@ static ulong load_serial_ymodem(ulong offset, int mode) } } + if (err) { + xyzModem_stream_terminate((err == xyzModem_cancel) ? false : true, &getcxmodem); + xyzModem_stream_close(&err); + printf("\n%s\n", xyzModem_error(err)); + return (~0); /* Download aborted */ + } + if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) efi_set_bootdev("Uart", "", "", map_sysmem(offset, 0), size); } else { - printf("%s\n", xyzModem_error(err)); + printf("\n%s\n", xyzModem_error(err)); + return (~0); /* Download aborted */ } xyzModem_stream_terminate(false, &getcxmodem); -- cgit v1.1 From c97b2557bcd5899cdf7bd57e09379b159f4796c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:43 +0200 Subject: xyz-modem: Show information about finished transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Show "## Start Addr" or "## Binary (...) download aborted" information like in Kermit "loadb" command. Signed-off-by: Pali Rohár --- cmd/load.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cmd/load.c b/cmd/load.c index 549e563..249ebd4 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -474,6 +474,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, addr = load_serial_ymodem(offset, xyzModem_ymodem); + if (addr == ~0) { + image_load_addr = 0; + printf("## Binary (ymodem) download aborted\n"); + rcode = 1; + } else { + printf("## Start Addr = 0x%08lX\n", addr); + image_load_addr = addr; + } } else if (strcmp(argv[0],"loadx")==0) { printf("## Ready for binary (xmodem) download " "to 0x%08lX at %d bps...\n", @@ -482,6 +490,14 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc, addr = load_serial_ymodem(offset, xyzModem_xmodem); + if (addr == ~0) { + image_load_addr = 0; + printf("## Binary (xmodem) download aborted\n"); + rcode = 1; + } else { + printf("## Start Addr = 0x%08lX\n", addr); + image_load_addr = addr; + } } else { printf("## Ready for binary (kermit) download " -- cgit v1.1 From dffeb400985d3244ce13ca95ad7c18a78ffd207f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Tue, 3 Aug 2021 16:28:44 +0200 Subject: xyz-modem: Allow to cancel transfer also by CTRL+C MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently it is possible to cancel loadx and loady commands by pressing CTRL+X (CAN character) at least 3 times quickly. All other U-Boot commands, including loadb and loads can be cancelled by CTRL+C. So allow it also in xyz-modem code used by loadx and loady commands. Implement it by handling CTRL+C (ETX character) in the same way as CTRL+X (CAN character). Due to how x/y-modem protocol works, it is required to press CTRL+C or CTRL+X at least 3 times quickly. Signed-off-by: Pali Rohár Reviewed-by: Simon Glass --- common/xyzModem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/xyzModem.c b/common/xyzModem.c index c200c9f..ece25ac 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -32,6 +32,7 @@ /* Values magic to the protocol */ #define SOH 0x01 #define STX 0x02 +#define ETX 0x03 /* ^C for interrupt */ #define EOT 0x04 #define ACK 0x06 #define BSP 0x08 @@ -283,6 +284,7 @@ xyzModem_get_hdr (void) hdr_found = true; break; case CAN: + case ETX: xyz.total_CAN++; ZM_DEBUG (zm_dump (__LINE__)); if (++can_total == xyzModem_CAN_COUNT) -- cgit v1.1