[PATCH] tftp: allocate at least 4096 bytes for FIFO

[PATCH] tftp: allocate at least 4096 bytes for FIFO

[Ahmad Fatoum]

On one board, boots from /mnt/tftp currently fail for me with:

ERROR: tftp: tftp: not enough room in kfifo (only 1376 out of 1432 written

This is overly annoying, because it doesn't abort the boot and thus an
incomplete image is started, that eventually crashes.

As this didn't happen with the previous hardcoded value of 4096, restore
working order by ensuring we have at least that many bytes in the kfifo.

Fixes: 480ed057aacb ("tftp: allocate buffers and fifo dynamically")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/tftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 2bffae2bf36e..e1c1b0e7269f 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -546,7 +546,7 @@ 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(max(priv->blocksize * priv->windowsize, 4096U));
 	if (!priv->fifo)
 		goto err;
 
-- 
2.30.2

Re: [PATCH] tftp: allocate at least 4096 bytes for FIFO

[Enrico Scholz]

Ahmad Fatoum <a.fatoum@pengutronix.de> writes:

> On one board, boots from /mnt/tftp currently fail for me with:
>
> ERROR: tftp: tftp: not enough room in kfifo (only 1376 out of 1432 written

Thanks for noticing that; I sent fixups for the recent patchset.

Perhaps, can you test it on your failing platform with changing

| #define TFTP_EXTRA_BLOCKS	2

to

| #define TFTP_EXTRA_BLOCKS	0

?

This should turn off the workaround of over-allocating memory and isolate
(and verify) the fix for the real problem.



Enrico

Re: [PATCH] tftp: allocate at least 4096 bytes for FIFO

[Ahmad Fatoum]

Hello Enrico,

On 05.09.22 11:03, Enrico Scholz wrote:
> Ahmad Fatoum <a.fatoum@pengutronix.de> writes:
> 
>> On one board, boots from /mnt/tftp currently fail for me with:
>>
>> ERROR: tftp: tftp: not enough room in kfifo (only 1376 out of 1432 written
> 
> Thanks for noticing that; I sent fixups for the recent patchset.

Thanks for the prompt reply and series.

> Perhaps, can you test it on your failing platform with changing
> 
> | #define TFTP_EXTRA_BLOCKS	2
> 
> to
> 
> | #define TFTP_EXTRA_BLOCKS	0
> 
> ?
> 
> This should turn off the workaround of over-allocating memory and isolate
> (and verify) the fix for the real problem.

Issue was not reproducible with TFTP_EXTRA_BLOCKS 0.

FWIW, the same barebox binary only had the issue when booting via USB network adapter
(DUB-E100) on one board, but not when booting from i.MX8MM FEC on another board.

Cheers,
Ahmad

> 
> 
> 
> Enrico
> 


-- 
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 |