mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] net.c: Don't forget about the first fragment.
@ 2023-07-04 17:55 Christian Melki
  2023-07-28  6:37 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Melki @ 2023-07-04 17:55 UTC (permalink / raw)
  To: barebox

It's possible to request very large messages using
the current code base. F.ex. UDP datagrams with the tftp client.
The tftp servers will happily reply with fragmented IP frames.
All these frame parts need to be dropped as BB currently doesn't
do fragment reassembly.

The current check was for fragment offsets only (0x1fff).
But the first frame has fragment offset 0 and would slip through
this check. That could result in a seemingly OK frame
for the tftp client, but with broken data.

Add check for the MF (More Fragments) flag. Should cover the
first packet too.

Signed-off-by: Christian Melki <christian.melki@t2data.com>
---
 net/net.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 19161d2e82..b842041d59 100644
--- a/net/net.c
+++ b/net/net.c
@@ -678,7 +678,12 @@ static int net_handle_ip(struct eth_device *edev, unsigned char *pkt, int len)
 	if ((ip->hl_v & 0xf0) != 0x40)
 		goto bad;
 
-	if (ip->frag_off & htons(0x1fff)) /* Can't deal w/ fragments */
+	/* Can't deal w/ fragments.
+	 * Ether a fragment offset (13 bits), or
+	 * MF (More Fragments) from frag. flags (3 bits).
+	 * MF - because first fragment has fragment offset 0
+	 */
+	if (ip->frag_off & htons(0x3fff)) 
 		goto bad;
 	if (!net_checksum_ok((unsigned char *)ip, sizeof(struct iphdr)))
 		goto bad;
-- 
2.34.1




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

* Re: [PATCH] net.c: Don't forget about the first fragment.
  2023-07-04 17:55 [PATCH] net.c: Don't forget about the first fragment Christian Melki
@ 2023-07-28  6:37 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2023-07-28  6:37 UTC (permalink / raw)
  To: Christian Melki; +Cc: barebox

On Tue, Jul 04, 2023 at 07:55:35PM +0200, Christian Melki wrote:
> It's possible to request very large messages using
> the current code base. F.ex. UDP datagrams with the tftp client.
> The tftp servers will happily reply with fragmented IP frames.
> All these frame parts need to be dropped as BB currently doesn't
> do fragment reassembly.
> 
> The current check was for fragment offsets only (0x1fff).
> But the first frame has fragment offset 0 and would slip through
> this check. That could result in a seemingly OK frame
> for the tftp client, but with broken data.
> 
> Add check for the MF (More Fragments) flag. Should cover the
> first packet too.
> 
> Signed-off-by: Christian Melki <christian.melki@t2data.com>
> ---
>  net/net.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 19161d2e82..b842041d59 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -678,7 +678,12 @@ static int net_handle_ip(struct eth_device *edev, unsigned char *pkt, int len)
>  	if ((ip->hl_v & 0xf0) != 0x40)
>  		goto bad;
>  
> -	if (ip->frag_off & htons(0x1fff)) /* Can't deal w/ fragments */
> +	/* Can't deal w/ fragments.
> +	 * Ether a fragment offset (13 bits), or
> +	 * MF (More Fragments) from frag. flags (3 bits).
> +	 * MF - because first fragment has fragment offset 0
> +	 */
> +	if (ip->frag_off & htons(0x3fff)) 

Nice catch ;)

Applied with typo fixed (Ether->Either)

Sascha

-- 
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] 2+ messages in thread

end of thread, other threads:[~2023-07-28  6:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-04 17:55 [PATCH] net.c: Don't forget about the first fragment Christian Melki
2023-07-28  6:37 ` Sascha Hauer

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