| Message ID | 20240415130013.26721-2-yasuharu.shibata@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | net: wget: fix TCP sequence number wrap around issue | expand |
Hi On Mon, Apr 15, 2024 at 3:01 PM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > > If tcp_seq_num is wrap around, tcp_seq_num >= initial_data_seq_num > isn't satisfied and store_block() isn't called. > The condition has a wrap around issue, so it is fixed in this patch. > > Signed-off-by: Yasuharu Shibata <yasuharu.shibata@gmail.com> > --- > net/wget.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/net/wget.c b/net/wget.c > index 71bac92d84..abab371e58 100644 > --- a/net/wget.c > +++ b/net/wget.c > @@ -404,9 +404,7 @@ static void wget_handler(uchar *pkt, u16 dport, > } > next_data_seq_num = tcp_seq_num + len; > > - if (tcp_seq_num >= initial_data_seq_num && > - store_block(pkt, tcp_seq_num - initial_data_seq_num, > - len) != 0) { > + if (store_block(pkt, tcp_seq_num - initial_data_seq_num, len) != 0) { > wget_fail("wget: store error\n", > tcp_seq_num, tcp_ack_num, action); > net_set_state(NETLOOP_FAIL); I think I have sent some time ago ;) Anyway look sane. I was having the same feeling on code inspection Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com> > -- > 2.25.1 >
Hi Michael, On Mon, 15 Apr 2024 at 22:03, Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > I think I have sent some time ago ;) > > Anyway look sane. I was having the same feeling on code inspection > > Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com> Thank you for your review. I already checked the thread, sorry I couldn't find your patch and I couldn't see whether it is the same. In any case, I consider there is a potential issue about wrap around, so I submitted a patch. -- Best regards, Yasuharu Shibata
Hi On Mon, Apr 15, 2024 at 3:48 PM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > > Hi Michael, > > On Mon, 15 Apr 2024 at 22:03, Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > > > > I think I have sent some time ago ;) > > > > Anyway look sane. I was having the same feeling on code inspection > > > > Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com> > > Thank you for your review. > I already checked the thread, sorry I couldn't find your patch and > I couldn't see whether it is the same. > In any case, I consider there is a potential issue about > wrap around, so I submitted a patch. > Very good job ;) to fix it. Just add Suggest-by: ;) Michael > -- > Best regards, > Yasuharu Shibata
Hi On Mon, Apr 15, 2024 at 3:55 PM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > Hi > > On Mon, Apr 15, 2024 at 3:48 PM Yasuharu Shibata > <yasuharu.shibata@gmail.com> wrote: > > > > Hi Michael, > > > > On Mon, 15 Apr 2024 at 22:03, Michael Nazzareno Trimarchi > > <michael@amarulasolutions.com> wrote: > > > > > > I think I have sent some time ago ;) > > > > > > Anyway look sane. I was having the same feeling on code inspection > > > > > > Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com> > > > > Thank you for your review. > > I already checked the thread, sorry I couldn't find your patch and > > I couldn't see whether it is the same. > > In any case, I consider there is a potential issue about > > wrap around, so I submitted a patch. > > > > Very good job ;) to fix it. Just add Suggest-by: ;) > https://lore.kernel.org/all/CAOMZO5AO5X3aHR0AyriijYa309USuA0HJ6oKrHTqQvhW7i8i9g@mail.gmail.com/T/ Mine was here Michael > Michael > > > -- > > Best regards, > > Yasuharu Shibata > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com
Hi Yasuharu, On Mon, Apr 15, 2024 at 10:01 AM Yasuharu Shibata <yasuharu.shibata@gmail.com> wrote: > > If tcp_seq_num is wrap around, tcp_seq_num >= initial_data_seq_num > isn't satisfied and store_block() isn't called. > The condition has a wrap around issue, so it is fixed in this patch. > > Signed-off-by: Yasuharu Shibata <yasuharu.shibata@gmail.com> Great work! I applied your previous patch: https://lore.kernel.org/u-boot/20240414104607.5966-1-yasuharu.shibata@gmail.com/ and this one against top-of-tree U-Boot and I no longer observe the wget corruption. Reported-by: Tim Harvey <tharvey@gateworks.com> Tested-by: Fabio Estevam <festevam@denx.de> Thanks a lot for fixing this long-standing wget bug. Cheers, Fabio Estevam
Hi Michael, On Mon, 15 Apr 2024 at 22:55, Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > Very good job ;) to fix it. Just add Suggest-by: ;) Thank you for your advice. I sent following v2 patch. https://lore.kernel.org/u-boot/20240416002624.1909-1-yasuharu.shibata@gmail.com/
diff --git a/net/wget.c b/net/wget.c index 71bac92d84..abab371e58 100644 --- a/net/wget.c +++ b/net/wget.c @@ -404,9 +404,7 @@ static void wget_handler(uchar *pkt, u16 dport, } next_data_seq_num = tcp_seq_num + len; - if (tcp_seq_num >= initial_data_seq_num && - store_block(pkt, tcp_seq_num - initial_data_seq_num, - len) != 0) { + if (store_block(pkt, tcp_seq_num - initial_data_seq_num, len) != 0) { wget_fail("wget: store error\n", tcp_seq_num, tcp_ack_num, action); net_set_state(NETLOOP_FAIL);
If tcp_seq_num is wrap around, tcp_seq_num >= initial_data_seq_num isn't satisfied and store_block() isn't called. The condition has a wrap around issue, so it is fixed in this patch. Signed-off-by: Yasuharu Shibata <yasuharu.shibata@gmail.com> --- net/wget.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)