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

[Richard Weinberger <richard@nod.at>]

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