[PATCH] net: wget: Avoid packet queue overflow

[PATCH] net: wget: Avoid packet queue overflow

[Richard Weinberger]

Make sure to stay within bounds, as a misbehaving HTTP server
can trigger a buffer overflow if not properly handled.

Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 net/wget.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/wget.c b/net/wget.c
index 2dbfeb1a1d5b..8bb4d72db1ae 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -35,7 +35,8 @@ struct pkt_qd {
  * The actual packet bufers are in the kernel space, and are
  * expected to be overwritten by the downloaded image.
  */
-static struct pkt_qd pkt_q[PKTBUFSRX / 4];
+#define PKTQ_SZ (PKTBUFSRX / 4)
+static struct pkt_qd pkt_q[PKTQ_SZ];
 static int pkt_q_idx;
 static unsigned long content_length;
 static unsigned int packets;
@@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int tcp_seq_num,
 		pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
 		pkt_q[pkt_q_idx].len = len;
 		pkt_q_idx++;
+
+		if (pkt_q_idx >= PKTQ_SZ) {
+			printf("wget: Fatal error, queue overrun!\n");
+			net_set_state(NETLOOP_FAIL);
+
+			return;
+		}
 	} else {
 		debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
 		/* sizeof(http_eom) - 1 is the string length of (http_eom) */
-- 
2.26.2

Re: [PATCH] net: wget: Avoid packet queue overflow

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "richard" <richard@nod.at>
> An: u-boot@lists.denx.de
> CC: "richard" <richard@nod.at>, "Joe Hershberger" <joe.hershberger@ni.com>, "Ramon Fried" <rfried.dev@gmail.com>
> Gesendet: Donnerstag, 20. Juli 2023 14:51:56
> Betreff: [PATCH] net: wget: Avoid packet queue overflow

> Make sure to stay within bounds, as a misbehaving HTTP server
> can trigger a buffer overflow if not properly handled.
> 
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> net/wget.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/wget.c b/net/wget.c
> index 2dbfeb1a1d5b..8bb4d72db1ae 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -35,7 +35,8 @@ struct pkt_qd {
>  * The actual packet bufers are in the kernel space, and are
>  * expected to be overwritten by the downloaded image.
>  */
> -static struct pkt_qd pkt_q[PKTBUFSRX / 4];
> +#define PKTQ_SZ (PKTBUFSRX / 4)
> +static struct pkt_qd pkt_q[PKTQ_SZ];
> static int pkt_q_idx;
> static unsigned long content_length;
> static unsigned int packets;
> @@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int
> tcp_seq_num,
> 		pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
> 		pkt_q[pkt_q_idx].len = len;
> 		pkt_q_idx++;
> +
> +		if (pkt_q_idx >= PKTQ_SZ) {
> +			printf("wget: Fatal error, queue overrun!\n");
> +			net_set_state(NETLOOP_FAIL);
> +
> +			return;
> +		}
> 	} else {
> 		debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
> 		/* sizeof(http_eom) - 1 is the string length of (http_eom) */

Friendly ping. :-)

Thanks,
//richard

Re: [PATCH] net: wget: Avoid packet queue overflow

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

On Thu, Aug 31, 2023 at 12:27:59PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "richard" <richard@nod.at>
> > An: u-boot@lists.denx.de
> > CC: "richard" <richard@nod.at>, "Joe Hershberger" <joe.hershberger@ni.com>, "Ramon Fried" <rfried.dev@gmail.com>
> > Gesendet: Donnerstag, 20. Juli 2023 14:51:56
> > Betreff: [PATCH] net: wget: Avoid packet queue overflow
> 
> > Make sure to stay within bounds, as a misbehaving HTTP server
> > can trigger a buffer overflow if not properly handled.
> > 
> > Cc: Joe Hershberger <joe.hershberger@ni.com>
> > Cc: Ramon Fried <rfried.dev@gmail.com>
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > ---
> > net/wget.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/wget.c b/net/wget.c
> > index 2dbfeb1a1d5b..8bb4d72db1ae 100644
> > --- a/net/wget.c
> > +++ b/net/wget.c
> > @@ -35,7 +35,8 @@ struct pkt_qd {
> >  * The actual packet bufers are in the kernel space, and are
> >  * expected to be overwritten by the downloaded image.
> >  */
> > -static struct pkt_qd pkt_q[PKTBUFSRX / 4];
> > +#define PKTQ_SZ (PKTBUFSRX / 4)
> > +static struct pkt_qd pkt_q[PKTQ_SZ];
> > static int pkt_q_idx;
> > static unsigned long content_length;
> > static unsigned int packets;
> > @@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int
> > tcp_seq_num,
> > 		pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
> > 		pkt_q[pkt_q_idx].len = len;
> > 		pkt_q_idx++;
> > +
> > +		if (pkt_q_idx >= PKTQ_SZ) {
> > +			printf("wget: Fatal error, queue overrun!\n");
> > +			net_set_state(NETLOOP_FAIL);
> > +
> > +			return;
> > +		}
> > 	} else {
> > 		debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
> > 		/* sizeof(http_eom) - 1 is the string length of (http_eom) */

This seems fine and I'll pick it up soon.  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] net: wget: Avoid packet queue overflow

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Tom Rini" <trini@konsulko.com>
> An: "richard" <richard@nod.at>
> CC: "u-boot" <u-boot@lists.denx.de>, "Joe Hershberger" <joe.hershberger@ni.com>, "Ramon Fried" <rfried.dev@gmail.com>
> Gesendet: Donnerstag, 31. August 2023 18:27:03
> Betreff: Re: [PATCH] net: wget: Avoid packet queue overflow

> On Thu, Aug 31, 2023 at 12:27:59PM +0200, Richard Weinberger wrote:
>> ----- Ursprüngliche Mail -----
>> > Von: "richard" <richard@nod.at>
>> > An: u-boot@lists.denx.de
>> > CC: "richard" <richard@nod.at>, "Joe Hershberger" <joe.hershberger@ni.com>,
>> > "Ramon Fried" <rfried.dev@gmail.com>
>> > Gesendet: Donnerstag, 20. Juli 2023 14:51:56
>> > Betreff: [PATCH] net: wget: Avoid packet queue overflow
>> 
>> > Make sure to stay within bounds, as a misbehaving HTTP server
>> > can trigger a buffer overflow if not properly handled.
>> > 
>> > Cc: Joe Hershberger <joe.hershberger@ni.com>
>> > Cc: Ramon Fried <rfried.dev@gmail.com>
>> > Signed-off-by: Richard Weinberger <richard@nod.at>
>> > ---
>> > net/wget.c | 10 +++++++++-
>> > 1 file changed, 9 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/net/wget.c b/net/wget.c
>> > index 2dbfeb1a1d5b..8bb4d72db1ae 100644
>> > --- a/net/wget.c
>> > +++ b/net/wget.c
>> > @@ -35,7 +35,8 @@ struct pkt_qd {
>> >  * The actual packet bufers are in the kernel space, and are
>> >  * expected to be overwritten by the downloaded image.
>> >  */
>> > -static struct pkt_qd pkt_q[PKTBUFSRX / 4];
>> > +#define PKTQ_SZ (PKTBUFSRX / 4)
>> > +static struct pkt_qd pkt_q[PKTQ_SZ];
>> > static int pkt_q_idx;
>> > static unsigned long content_length;
>> > static unsigned int packets;
>> > @@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int
>> > tcp_seq_num,
>> > 		pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
>> > 		pkt_q[pkt_q_idx].len = len;
>> > 		pkt_q_idx++;
>> > +
>> > +		if (pkt_q_idx >= PKTQ_SZ) {
>> > +			printf("wget: Fatal error, queue overrun!\n");
>> > +			net_set_state(NETLOOP_FAIL);
>> > +
>> > +			return;
>> > +		}
>> > 	} else {
>> > 		debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
>> > 		/* sizeof(http_eom) - 1 is the string length of (http_eom) */
> 
> This seems fine and I'll pick it up soon.  Thanks!

Is there something I can do to help this merged?

Thanks,
//richard

Re: [PATCH] net: wget: Avoid packet queue overflow

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Thu, Jul 20, 2023 at 02:51:56PM +0200, Richard Weinberger wrote:

> Make sure to stay within bounds, as a misbehaving HTTP server
> can trigger a buffer overflow if not properly handled.
> 
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Richard Weinberger <richard@nod.at>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]