mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] tftp: allocate at least 4096 bytes for FIFO
@ 2022-09-05  7:00 Ahmad Fatoum
  2022-09-05  9:03 ` Enrico Scholz
  0 siblings, 1 reply; 3+ messages in thread
From: Ahmad Fatoum @ 2022-09-05  7:00 UTC (permalink / raw)
  To: barebox; +Cc: Enrico Scholz, 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




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tftp: allocate at least 4096 bytes for FIFO
  2022-09-05  7:00 [PATCH] tftp: allocate at least 4096 bytes for FIFO Ahmad Fatoum
@ 2022-09-05  9:03 ` Enrico Scholz
  2022-09-05 12:07   ` Ahmad Fatoum
  0 siblings, 1 reply; 3+ messages in thread
From: Enrico Scholz @ 2022-09-05  9:03 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tftp: allocate at least 4096 bytes for FIFO
  2022-09-05  9:03 ` Enrico Scholz
@ 2022-09-05 12:07   ` Ahmad Fatoum
  0 siblings, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2022-09-05 12:07 UTC (permalink / raw)
  To: Enrico Scholz; +Cc: barebox

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 |



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-09-05 15:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05  7:00 [PATCH] tftp: allocate at least 4096 bytes for FIFO Ahmad Fatoum
2022-09-05  9:03 ` Enrico Scholz
2022-09-05 12:07   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox