[PATCH 0/3] tftp fixups

[PATCH 0/3] tftp fixups

[Enrico Scholz]

Fixups for the "tftp" branch in next.

Enrico Scholz (3):
  fixup! tftp: implement 'windowsize' (RFC 7440) support
  fixup! tftp: detect out-of-memory situations
  tftp: make read() fail in error case

 fs/tftp.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

-- 
2.37.2

[PATCH 1/3] fixup! tftp: implement 'windowsize' (RFC 7440) support

[Enrico Scholz]

avoid fifo overflow on read() with small buffer sizes.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 fs/tftp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 2bffae2bf36e..a3fe7dfd590e 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -78,6 +78,9 @@
 /* size of cache which deals with udp reordering */
 #define TFTP_WINDOW_CACHE_NUM	CONFIG_FS_TFTP_REORDER_CACHE_SIZE
 
+/* allocate this number of blocks more than needed in the fifo */
+#define TFTP_EXTRA_BLOCKS	2
+
 /* marker for an emtpy 'tftp_cache' */
 #define TFTP_CACHE_NO_ID	(-1)
 
@@ -546,7 +549,8 @@ static int tftp_allocate_transfer(struct file_priv *priv)
 
 	/* multiplication is safe; both operands were checked in tftp_parse_oack()
 	   and are small integers */
-	priv->fifo = kfifo_alloc(priv->blocksize * priv->windowsize);
+	priv->fifo = kfifo_alloc(priv->blocksize *
+				 (priv->windowsize + TFTP_EXTRA_BLOCKS));
 	if (!priv->fifo)
 		goto err;
 
@@ -1025,7 +1029,12 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 		if (priv->state == STATE_DONE)
 			return outsize;
 
-		if (priv->last_block == priv->ack_block)
+		/* send the ACK only when fifo has been nearly depleted; else,
+		   when tftp_read() is called with small 'insize' values, it
+		   is possible that there is read more data from the network
+		   than consumed by kfifo_get() and the fifo overflows */
+		if (priv->last_block == priv->ack_block &&
+		    kfifo_len(priv->fifo) <= TFTP_EXTRA_BLOCKS * priv->blocksize)
 			tftp_send(priv);
 
 		ret = tftp_poll(priv);
-- 
2.37.2

[PATCH 2/3] fixup! tftp: detect out-of-memory situations

[Enrico Scholz]

minor typo fix

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 fs/tftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index a3fe7dfd590e..9a3753e50e37 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -591,7 +591,7 @@ static void tftp_put_data(struct file_priv *priv, uint16_t block,
 	sz = kfifo_put(priv->fifo, pkt, len);
 
 	if (sz != len) {
-		pr_err("tftp: not enough room in kfifo (only %u out of %zu written\n",
+		pr_err("tftp: not enough room in kfifo (only %u out of %zu written)\n",
 		       sz, len);
 		priv->err = -ENOMEM;
 		priv->state = STATE_DONE;
-- 
2.37.2

[PATCH 3/3] tftp: make read() fail in error case

[Enrico Scholz]

when tftp transfer goes in error state e.g. due to error packets sent
from the server or (unexpected) internal problems, let the read() fail
instead of ignoring these errors silently and corrupting the output.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 fs/tftp.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 9a3753e50e37..2592da94dd34 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -1017,7 +1017,7 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 {
 	struct file_priv *priv = f->priv;
 	size_t outsize = 0, now;
-	int ret;
+	int ret = 0;
 
 	pr_vdebug("%s %zu\n", __func__, insize);
 
@@ -1026,8 +1026,11 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 		outsize += now;
 		buf += now;
 		insize -= now;
-		if (priv->state == STATE_DONE)
-			return outsize;
+
+		if (priv->state == STATE_DONE) {
+			ret = priv->err;
+			break;
+		}
 
 		/* send the ACK only when fifo has been nearly depleted; else,
 		   when tftp_read() is called with small 'insize' values, it
@@ -1041,9 +1044,12 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 		if (ret == TFTP_ERR_RESEND)
 			tftp_send(priv);
 		if (ret < 0)
-			return ret;
+			break;
 	}
 
+	if (ret < 0)
+		return ret;
+
 	return outsize;
 }
 
-- 
2.37.2

Re: [PATCH 0/3] tftp fixups

[Ahmad Fatoum]

On 05.09.22 10:56, Enrico Scholz wrote:
> Fixups for the "tftp" branch in next.
> 
> Enrico Scholz (3):
>   fixup! tftp: implement 'windowsize' (RFC 7440) support
>   fixup! tftp: detect out-of-memory situations
>   tftp: make read() fail in error case

This fixes the issue of kfifo underflow I run into.

Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thanks,
Ahmad

> 
>  fs/tftp.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/3] tftp fixups

[Sascha Hauer]

On Mon, Sep 05, 2022 at 10:56:55AM +0200, Enrico Scholz wrote:
> Fixups for the "tftp" branch in next.
> 
> Enrico Scholz (3):
>   fixup! tftp: implement 'windowsize' (RFC 7440) support
>   fixup! tftp: detect out-of-memory situations
>   tftp: make read() fail in error case
> 
>  fs/tftp.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)

Applied, thanks

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |