Fix some Zmodem buffer sizing issues

4 files changed, 30 insertions, 10 deletions

diff --git a/apps/system/zmodem/Kconfig b/apps/system/zmodem/Kconfig
index a622e9e8e..7d84d1812 100644
--- a/apps/system/zmodem/Kconfig
+++ b/apps/system/zmodem/Kconfig
@@ -22,15 +22,22 @@ config SYSTEM_ZMODEM_RCVBUFSIZE
 	int "Receive buffer size"
 	default 512
 	---help---
-		The size of one buffer used to read data from the remote peer.
+		The size of one buffer used to read data from the remote peer.  The
+		total buffering capability is SYSTEM_ZMODEM_RCVBUFSIZE plus the size
+		of the RX buffer in the device driver.  If you are using a serial
+		driver with, say, USART0.  That that buffering capability includes
+		USART0_RXBUFFERSIZE.  This total buffering capability must be
+		significantly larger than SYSTEM_ZMODEM_PKTBUFSIZE (larger due
+		streaming race conditions, data expansion due to escaping, and
+		possible protocol overhead).
 
 config SYSTEM_ZMODEM_PKTBUFSIZE
 	int "Maximum packet size"
 	default 512
 	---help---
 		Data may be received in gulps of varying size and alignment.
-		Received packets data is properly packed into a packet buffer of
-		this size.
+		Received packets data is properly unescaped, aligned and packed
+		into a packet buffer of this size.
 
 config SYSTEM_ZMODEM_SNDBUFSIZE
 	int "Send buffer size"
diff --git a/apps/system/zmodem/zm.h b/apps/system/zmodem/zm.h
index 2dbafc9e4..1f8201469 100644
--- a/apps/system/zmodem/zm.h
+++ b/apps/system/zmodem/zm.h
@@ -210,6 +210,12 @@
 
 #define ZM_XFRDONE    1              /* Success - Transfer complete */
 
+/* The actual packet buffer size includes 5 bytes to hold the transfer type
+ * and the maxmimum size 4-byte CRC.
+ */
+
+#define ZM_PKTBUFSIZE (CONFIG_SYSTEM_ZMODEM_PKTBUFSIZE + 5)
+
 /* Debug Definitions ********************************************************/
 
 /* Non-standard debug selectable with CONFIG_DEBUG_ZMODEM.  Debug output goes
@@ -347,7 +353,7 @@ struct zm_state_s
    */
 
   uint8_t  rcvbuf[CONFIG_SYSTEM_ZMODEM_RCVBUFSIZE];
-  uint8_t  pktbuf[CONFIG_SYSTEM_ZMODEM_PKTBUFSIZE];
+  uint8_t  pktbuf[ZM_PKTBUFSIZE];
   uint8_t  scratch[CONFIG_SYSTEM_ZMODEM_SNDBUFSIZE];
 };
 
diff --git a/apps/system/zmodem/zm_receive.c b/apps/system/zmodem/zm_receive.c
index c26d15e02..e172b5740 100644
--- a/apps/system/zmodem/zm_receive.c
+++ b/apps/system/zmodem/zm_receive.c
@@ -324,8 +324,8 @@ static int zmr_zrinit(FAR struct zm_state_s *pzm)
   /* Send ZRINIT */
 
   pzm->timeout = CONFIG_SYSTEM_ZMODEM_RESPTIME;
-  buf[0]       = CONFIG_SYSTEM_ZMODEM_RCVBUFSIZE & 0xff;
-  buf[1]       = (CONFIG_SYSTEM_ZMODEM_RCVBUFSIZE >> 8) & 0xff;
+  buf[0]       = CONFIG_SYSTEM_ZMODEM_PKTBUFSIZE & 0xff;
+  buf[1]       = (CONFIG_SYSTEM_ZMODEM_PKTBUFSIZE >> 8) & 0xff;
   buf[2]       = 0;
   buf[3]       = pzmr->rcaps;
   return zm_sendhexhdr(pzm, ZRINIT, buf);
@@ -739,11 +739,13 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
           zmdbg("PSTATE %d:%d->%d:%d\n",
                 pzm->pstate, pzm->psubstate, PSTATE_DATA, PDATA_READ);
 
-          /* Revert to the IDLE state and send the cancel string */
+          /* Send the cancel string */
 
-          pzm->pstate    = PSTATE_DATA;
-          pzm->psubstate = PDATA_READ;
           (void)zm_remwrite(pzm->remfd, g_canistr, CANISTR_SIZE);
+
+          /* Enter PSTATE_DATA */
+
+          zm_readstate(pzm);
           return -EIO;
         }
       else
diff --git a/apps/system/zmodem/zm_state.c b/apps/system/zmodem/zm_state.c
index 261dba388..be0285d85 100644
--- a/apps/system/zmodem/zm_state.c
+++ b/apps/system/zmodem/zm_state.c
@@ -56,6 +56,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <ctype.h>
+#include <ctype.h>
 #include <fcntl.h>
 #include <sched.h>
 #include <assert.h>
@@ -641,9 +642,13 @@ static int zm_data(FAR struct zm_state_s *pzm, uint8_t ch)
 
   /* Make sure that there is space for another byte in the packet buffer */
 
-  if (pzm->pktlen >= CONFIG_SYSTEM_ZMODEM_PKTBUFSIZE)
+  if (pzm->pktlen >= ZM_PKTBUFSIZE)
     {
       zmdbg("ERROR:  The packet buffer is full\n");
+      zmdbg("        ch=%c[%02x] pktlen=%d ptktype=%02x ncrc=%d\n",
+            isprint(ch) ? ch : '.', ch, pzm->pktlen, pzm->pkttype, pzm->ncrc);
+      zmdbg("        rcvlen=%d rcvndx=%d\n",
+            pzm->rcvlen, pzm->rcvndx);
       return -ENOSPC;
     }