Hi Subhakar,

Thanks for the review.

On Fri, Jul 11, 2025 at 11:17:00AM +0530, Sudhakar Kuppusamy wrote:
> 
> 
> > On 11 Jul 2025, at 9:37 AM, Michael Chang via Grub-devel 
> > <grub-devel@gnu.org> wrote:
> > 
> > GRUB's TCP stack assigns source ports for outgoing connections starting
> > at 21550 and increments sequentially by 1 (e.g., 21550, 21551, ...).
> > While this generally works, it can lead to failures if the system
> > reboots rapidly and reuses the same source port too soon.
> > 
> > This issue was observed on powerpc-ieee1275 platforms using CAS (Client
> > Architecture Support) reboot. In such cases, loading the initrd over
> > HTTP may fail with connection timeouts. Packet captures show the failed
> > connections are flagged as "TCP Port Number Reused" by Wireshark.
> > 
> > The root cause is that GRUB reuses the same port shortly after reboot,
> > while the server may still be tracking the previous connection in
> > TIME_WAIT. This can result in the server rejecting the connection
> > attempt or responding with a stale ACK or RST, leading to handshake
> > failure.
> > 
> > This patch fixes the issue by introducing a time based source port
> > selection strategy. Instead of always starting from port 21550, GRUB now
> > computes an initial base port based on the current RTC time, divided
> > into 5 minute windows. The purpose of this time based strategy is to
> > ensure that GRUB avoids reusing the same source port within a 5 minute
> > window, thereby preventing collisions with stale server side connection
> > tracking that could interfere with a new TCP handshake.
> > 
> > A step size of 8 ensures that the same port will not be reused across
> > reboots unless GRUB opens more than 8 TCP connections per second on
> > average, something that is highly unlikely. In typical usage, a GRUB
> > boot cycle lasts about 15 seconds and may open fewer than 100
> > connections total, well below the reuse threshold. This makes the
> > approach robust against short reboot intervals while keeping the logic
> > simple and deterministic.
> > 
> > Signed-off-by: Michael Chang <mch...@suse.com>
> > ---
> > grub-core/net/tcp.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 38 insertions(+), 1 deletion(-)
> > 
> > diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c
> > index 93dee0caa..d0589c458 100644
> > --- a/grub-core/net/tcp.c
> > +++ b/grub-core/net/tcp.c
> > @@ -22,6 +22,7 @@
> > #include <grub/net/netbuff.h>
> > #include <grub/time.h>
> > #include <grub/priority_queue.h>
> > +#include <grub/datetime.h>
> > 
> > #define TCP_SYN_RETRANSMISSION_TIMEOUT GRUB_NET_INTERVAL
> > #define TCP_SYN_RETRANSMISSION_COUNT GRUB_NET_TRIES
> > @@ -552,6 +553,36 @@ grub_net_tcp_accept (grub_net_tcp_socket_t sock,
> >   return GRUB_ERR_NONE;
> > }
> > 
> > +/*
> > + * Derive a time-based source port to avoid reusing the same port across
> > + * reboots. This helps prevent failures caused by server side TCP state 
> > (e.g.
> > + * TIME_WAIT) from interfering with new connections using the same socket.
> > + *
> > + * The base port starts at 21550 and increments every second by 8 across a 
> > 5
> > + * minute window (300 seconds), giving 2400 possible distinct base ports 
> > per
> > + * window. In typical GRUB usage, the number of connections per boot is 
> > small,
> > + * so reuse is effectively avoided.
> > + */
> > +static grub_uint16_t
> > +get_initial_base_port (void)
> > +{
> > +  grub_err_t err;
> > +  struct grub_datetime date;
> > +  grub_int64_t t = 0;
> > +  grub_uint64_t r = 0;
> > +
> > +  err = grub_get_datetime (&date);
> > +  if (err || !grub_datetime2unixtime (&date, &t))
> 
> I would suggest to use enum to check err
> 
> err != GRUB_ERR_NONE

OK. I'll change to your suggestion.

> 
> > +    {
> > +      grub_errno = GRUB_ERR_NONE;
> > +      return 21550;
> > +    }
> > +
> > +  grub_divmod64 (t, 300, &r);
> > +
> > +  return 21550 + (r << 3);
> > +}
> > +
> > grub_net_tcp_socket_t
> > grub_net_tcp_open (char *server,
> >                grub_uint16_t out_port,
> > @@ -569,13 +600,19 @@ grub_net_tcp_open (char *server,
> >   struct grub_net_network_level_interface *inf;
> >   grub_net_network_level_address_t gateway;
> >   grub_net_tcp_socket_t socket;
> > -  static grub_uint16_t in_port = 21550;
> > +  static grub_uint16_t in_port;
> >   struct grub_net_buff *nb;
> >   struct tcphdr *tcph;
> >   int i;
> >   grub_uint8_t *nbd;
> >   grub_net_link_level_address_t ll_target_addr;
> > 
> > +  if (!in_port)
> > +    {
> > +      in_port = get_initial_base_port ();
> > +      grub_dprintf ("net", "base port: %d\n", in_port);
> > +    }
> > +
> >   err = grub_net_resolve_address (server, &addr);
> >   if (err)
> >     return NULL;
> > -- 
> > 2.50.0
> > 
> > 
> 
> Reviewed-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>

I'll update next version and add your Reviewed-by.

Thanks,
Michael

> 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to