mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
To: barebox@lists.infradead.org
Cc: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Subject: [PATCH v4 10/21] tftp: allocate buffers and fifo dynamically
Date: Tue, 30 Aug 2022 09:38:05 +0200	[thread overview]
Message-ID: <20220830073816.2694734-11-enrico.scholz@sigma-chemnitz.de> (raw)
In-Reply-To: <20220830073816.2694734-1-enrico.scholz@sigma-chemnitz.de>

Use the actual blocksize for allocating buffers instead of assuming an
hardcoded value.

This requires to add an additional 'START' state which is entered
after receiving (RRQ) or sending (WRQ) the OACK.  Without it, the next
state would be entered and the (not allocated yet) fifo be used.

For non-rfc 2347 servers (which do not understand OACK and start with
data transfer immediately after RRQ/WRQ), additional transitions in
the state machine were implemented.

Code adds some sanity checks in the new code paths.

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

diff --git a/fs/tftp.c b/fs/tftp.c
index 07de8334f202..64a94797cda3 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -59,14 +59,15 @@
 #define STATE_WRQ	2
 #define STATE_RDATA	3
 #define STATE_WDATA	4
-#define STATE_OACK	5
+/* OACK from server has been received and we can begin to sent either the ACK
+   (for RRQ) or data (for WRQ) */
+#define STATE_START	5
 #define STATE_WAITACK	6
 #define STATE_LAST	7
 #define STATE_DONE	8
 
 #define TFTP_BLOCK_SIZE		512	/* default TFTP block size */
 #define TFTP_MTU_SIZE		1432	/* MTU based block size */
-#define TFTP_FIFO_SIZE		4096
 
 #define TFTP_ERR_RESEND	1
 
@@ -111,10 +112,10 @@ static char const * const tftp_states[] = {
 	[STATE_WRQ] = "WRQ",
 	[STATE_RDATA] = "RDATA",
 	[STATE_WDATA] = "WDATA",
-	[STATE_OACK] = "OACK",
 	[STATE_WAITACK] = "WAITACK",
 	[STATE_LAST] = "LAST",
 	[STATE_DONE] = "DONE",
+	[STATE_START] = "START",
 };
 
 static int tftp_send(struct file_priv *priv)
@@ -166,7 +167,6 @@ static int tftp_send(struct file_priv *priv)
 	case STATE_RDATA:
 		if (priv->block == priv->block_requested)
 			return 0;
-	case STATE_OACK:
 		xp = pkt;
 		s = (uint16_t *)pkt;
 		*s++ = htons(TFTP_ACK);
@@ -261,11 +261,39 @@ static void tftp_timer_reset(struct file_priv *priv)
 	priv->progress_timeout = priv->resend_timeout = get_time_ns();
 }
 
+static int tftp_allocate_transfer(struct file_priv *priv)
+{
+	debug_assert(!priv->fifo);
+	debug_assert(!priv->buf);
+
+	priv->fifo = kfifo_alloc(priv->blocksize);
+	if (!priv->fifo)
+		goto err;
+
+	if (priv->push) {
+		priv->buf = xmalloc(priv->blocksize);
+		if (!priv->buf) {
+			kfifo_free(priv->fifo);
+			priv->fifo = NULL;
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	priv->err = -ENOMEM;
+	priv->state = STATE_DONE;
+
+	return priv->err;
+}
+
 static void tftp_recv(struct file_priv *priv,
 			uint8_t *pkt, unsigned len, uint16_t uh_sport)
 {
 	uint16_t opcode;
 	uint16_t block;
+	int rc;
 
 	/* according to RFC1350 minimal tftp packet length is 4 bytes */
 	if (len < 4)
@@ -294,48 +322,64 @@ static void tftp_recv(struct file_priv *priv,
 			break;
 		}
 
-		priv->block = block + 1;
-		tftp_timer_reset(priv);
+		switch (priv->state) {
+		case STATE_WRQ:
+			priv->tftp_con->udp->uh_dport = uh_sport;
+			priv->state = STATE_START;
+			break;
 
-		if (priv->state == STATE_LAST) {
+		case STATE_WAITACK:
+			priv->state = STATE_WDATA;
+			break;
+
+		case STATE_LAST:
 			priv->state = STATE_DONE;
 			break;
+
+		default:
+			pr_warn("ACK packet in %s state\n",
+				tftp_states[priv->state]);
+			goto ack_out;
 		}
-		priv->tftp_con->udp->uh_dport = uh_sport;
-		priv->state = STATE_WDATA;
+
+		priv->block = block + 1;
+		tftp_timer_reset(priv);
+
+	ack_out:
 		break;
 
 	case TFTP_OACK:
 		tftp_parse_oack(priv, pkt, len);
 		priv->tftp_con->udp->uh_dport = uh_sport;
-
-		if (priv->push) {
-			/* send first block */
-			priv->state = STATE_WDATA;
-			priv->block = 1;
-		} else {
-			/* send ACK */
-			priv->state = STATE_OACK;
-			priv->block = 0;
-			tftp_send(priv);
-		}
-
+		priv->state = STATE_START;
 		break;
+
 	case TFTP_DATA:
 		len -= 2;
 		priv->block = ntohs(*(uint16_t *)pkt);
 
-		if (priv->state == STATE_RRQ || priv->state == STATE_OACK) {
-			/* first block received */
-			priv->state = STATE_RDATA;
+		if (priv->state == STATE_RRQ) {
+			/* first block received; entered only with non rfc
+			   2347 (TFTP Option extension) compliant servers */
 			priv->tftp_con->udp->uh_dport = uh_sport;
+			priv->state = STATE_RDATA;
 			priv->last_block = 0;
+
+			rc = tftp_allocate_transfer(priv);
+			if (rc < 0)
+				break;
 		}
 
 		if (priv->block == priv->last_block)
 			/* Same block again; ignore it. */
 			break;
 
+		if (len > priv->blocksize) {
+			pr_warn("tftp: oversized packet (%u > %d) received\n",
+				len, priv->blocksize);
+			break;
+		}
+
 		priv->last_block = priv->block;
 
 		tftp_timer_reset(priv);
@@ -378,6 +422,30 @@ static void tftp_handler(void *ctx, char *packet, unsigned len)
 	tftp_recv(priv, pkt, net_eth_to_udplen(packet), udp->uh_sport);
 }
 
+static int tftp_start_transfer(struct file_priv *priv)
+{
+	int rc;
+
+	rc = tftp_allocate_transfer(priv);
+	if (rc < 0)
+		/* function sets 'priv->state = STATE_DONE' and 'priv->err' in
+		   error case */
+		return rc;
+
+	if (priv->push) {
+		/* send first block */
+		priv->state = STATE_WDATA;
+		priv->block = 1;
+	} else {
+		/* send ACK */
+		priv->state = STATE_RDATA;
+		priv->block = 0;
+		tftp_send(priv);
+	}
+
+	return 0;
+}
+
 static struct file_priv *tftp_do_open(struct device_d *dev,
 		int accmode, struct dentry *dentry)
 {
@@ -409,48 +477,74 @@ static struct file_priv *tftp_do_open(struct device_d *dev,
 	priv->blocksize = TFTP_BLOCK_SIZE;
 	priv->block_requested = -1;
 
-	priv->fifo = kfifo_alloc(TFTP_FIFO_SIZE);
-	if (!priv->fifo) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
 	parseopt_hu(fsdev->options, "port", &port);
 
 	priv->tftp_con = net_udp_new(tpriv->server, port, tftp_handler, priv);
 	if (IS_ERR(priv->tftp_con)) {
 		ret = PTR_ERR(priv->tftp_con);
-		goto out1;
+		goto out;
 	}
 
 	ret = tftp_send(priv);
 	if (ret)
-		goto out2;
+		goto out1;
 
 	tftp_timer_reset(priv);
-	while (priv->state != STATE_RDATA &&
-			priv->state != STATE_DONE &&
-			priv->state != STATE_WDATA) {
-		ret = tftp_poll(priv);
-		if (ret == TFTP_ERR_RESEND)
-			tftp_send(priv);
-		if (ret < 0)
-			goto out2;
-	}
 
-	if (priv->state == STATE_DONE && priv->err) {
-		ret = priv->err;
-		goto out2;
-	}
+	/* - 'ret < 0'  ... error
+	   - 'ret == 0' ... further tftp_poll() required
+	   - 'ret == 1' ... startup finished */
+	do {
+		switch (priv->state) {
+		case STATE_DONE:
+			/* branch is entered in two situations:
+			   - non rfc 2347 compliant servers finished the
+			     transfer by sending a small file
+			   - some error occurred */
+			if (priv->err < 0)
+				ret = priv->err;
+			else
+				ret = 1;
+			break;
 
-	priv->buf = xmalloc(priv->blocksize);
+		case STATE_START:
+			ret = tftp_start_transfer(priv);
+			if (!ret)
+				ret = 1;
+			break;
+
+		case STATE_RDATA:
+			/* first data block of non rfc 2347 servers */
+			ret = 1;
+			break;
+
+		case STATE_RRQ:
+		case STATE_WRQ:
+			ret = tftp_poll(priv);
+			if (ret == TFTP_ERR_RESEND) {
+				tftp_send(priv);
+				ret = 0;
+			}
+			break;
+
+		default:
+			debug_assert(false);
+			break;
+		}
+	} while (ret == 0);
+
+	if (ret < 0)
+		goto out1;
 
 	return priv;
-out2:
-	net_unregister(priv->tftp_con);
 out1:
-	kfifo_free(priv->fifo);
+	net_unregister(priv->tftp_con);
 out:
+	if (priv->fifo)
+		kfifo_free(priv->fifo);
+
+	free(priv->filename);
+	free(priv->buf);
 	free(priv);
 
 	return ERR_PTR(ret);
@@ -567,7 +661,7 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 		if (priv->state == STATE_DONE)
 			return outsize;
 
-		if (TFTP_FIFO_SIZE - kfifo_len(priv->fifo) >= priv->blocksize)
+		if (kfifo_len(priv->fifo) == 0)
 			tftp_send(priv);
 
 		ret = tftp_poll(priv);
-- 
2.37.2




  parent reply	other threads:[~2022-08-30  7:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-30  7:37 [PATCH v4 00/21] add "windowsize" (RFC 7440) support for tftp Enrico Scholz
2022-08-30  7:37 ` [PATCH v4 01/21] tftp: add some 'const' annotations Enrico Scholz
2022-08-30  7:37 ` [PATCH v4 02/21] tftp: allow to change tftp port Enrico Scholz
2022-08-30  7:37 ` [PATCH v4 03/21] cmd:tftp: add '-P' option to set tftp server port number Enrico Scholz
2022-08-30  7:37 ` [PATCH v4 04/21] tftp: do not set 'tsize' in WRQ requests Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 05/21] tftp: assign 'priv->block' later in WRQ Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 06/21] tftp: minor refactoring of RRQ/WRQ packet generation code Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 07/21] tftp: replace hardcoded blksize by global constant Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 08/21] tftp: remove sanity check of first block Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 09/21] tftp: add debug_assert() macro Enrico Scholz
2022-08-30  7:38 ` Enrico Scholz [this message]
2022-08-30  7:38 ` [PATCH v4 11/21] tftp: add sanity check for OACK response Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 12/21] tftp: record whether tftp file is opened for lookup operation only Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 13/21] tftp: reduce block size on lookup requests Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 14/21] tftp: refactor data processing Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 15/21] tftp: detect out-of-memory situations Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 16/21] tftp: implement 'windowsize' (RFC 7440) support Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 17/21] tftp: do not use 'priv->block' for RRQ Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 18/21] tftp: reorder tftp packets Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 19/21] tftp: add selftest Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 20/21] tftp: accept OACK + DATA datagrams only in certain states Enrico Scholz
2022-08-30  7:38 ` [PATCH v4 21/21] tftp: add some documentation about windowsize support Enrico Scholz
2022-08-31  6:40 ` [PATCH v4 00/21] add "windowsize" (RFC 7440) support for tftp Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220830073816.2694734-11-enrico.scholz@sigma-chemnitz.de \
    --to=enrico.scholz@sigma-chemnitz.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox