kwboot : bug when board stops acking

[Tony Dinh]

Hello,

On Fri, Jan 6, 2023 at 9:52 AM Pali Rohár <pali at kernel.org> wrote:
>
> Hello Stephane!
>
> On Wednesday 08 January 2020 15:21:12 SC wrote:
> > Hi list,
> >
> > I think there's a bug in kwboot.
> > I have a faulty board that stops acking xmodem frames after a certain number of packets and kwboot
> > instead of exiting continues to send frames up to 100%.
> > With the one second retry timer it's very long! More than 2 hours :-(
>
> I saw very similar bugs in kwboot and together with Marek we fixed lot
> of issues in kwboot.
>
> Could you test if last version of kwboot from U-Boot git master branch
> has still same or similar issue?
>
> > The bug is in the kwboot_xm_sendblock() fonction :
> > https://gitlab.denx.de/u-boot/u-boot/blob/master/tools/kwboot.c#L375
> > kwboot_tty_recv detects the timeout but the return value is based on the last character received
> > (line 403)
> > The variable c is not initialized so if it keeps the previous value which is ACK => rc=0 => infinite
> > loop
> >
> > There are several ways to fix the bug:
> > * Surround the "switch(c)" with a test:
> >   if ( !((rc == -1) && (errno == ETIMEDOUT)) ) {
> >          rc = -1;
> >          switch (c) {
> >          case ACK:
> >              rc = 0;
> >              break;
> >          case NAK:
> >              errno = EBADMSG;
> >              break;
> >          case CAN:
> >              errno = ECANCELED;
> >              break;
> >          default:
> >              errno = EPROTO;
> >              break;
> >          }
> >      }
> >      return rc;
> > * In the case of a TIMEOUT set c to a specific value and add the case in the switch
> >

I've been running the latest kwboot (2023.01-rc4), and I did not see
this problem. And the current code looks OK to me. c is modified
before it is used later:
https://github.com/u-boot/u-boot/blob/master/tools/kwboot.c#L1238

And the function kwboot_xm_sendblock() in GitHub is line L1215!
https://github.com/u-boot/u-boot/blob/master/tools/kwboot.c#L1215

Probably Stephane is running an older version of kwboot?

Best,
Tony

> > Do you agree with my diagnosis?
> > What is the best solution to fix this bug?
> > Should I provide a patch?
> >
> > Thank you,
> > Stephane