[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) */

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

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!

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

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!
participants (2)
-
Richard Weinberger
-
Tom Rini