[Bugfix rtems-lwip 4/4] Fix crash due to link_detect_thread

[Kinsey Moore]

The intent of this patch is fine, but changes should be purely additive and
properly wrapped in the __rtems__ check. The addition of thread startup at
the end should also be wrapped in !NO_SYS.

Kinsey

On Sun, Mar 31, 2024 at 5:49 PM Bernd Moessner <berndmoessner80 at gmail.com>
wrote:

> Within xemac_add, the link_detect_thread is set up before the ethernet
> interface is configured and all data structures have been set up correctly.
> The steps within xemac_add are basically:
> 1) Set up link_detect_thread
> 2) Initialize the interface
> 3) Set up the link speed / start autonegotiation
> 4) When autonegotiate has completed, set up the data structures
>
> The issue is here that, for example if no cable is connected, the
> autonegotiate sequence will suspend the calling task. As soon as
> this happens, the link_detect_thread gets into running state and
> dereferences the data structures that havent been set up. Unfortunatelly,
> it isnt as easy as checking the pointer to the relevant data structure
> for being NULL within the link_detect_thread. Not going into details,
> the pointer (netif->state) isnt NULL in this moment of time, but it
> has to point to something completely different.
>
> It seems to me that the easiest solution to this problem is to reorder
> the sequence within xemac_add. Initialize the interface and set up the
> data structures, then create the link_detect_thread which requires all of
> this. After applying this patch, the sequence becomes:
>
> 1) Initialize the interface
> 2) Set up the link speed / start autonegotiation
> 3) When autonegotiate has completed, set up the data structures
> 4) Set up link_detect_thread
> ---
>  .../src/contrib/ports/xilinx/netif/xadapter.c | 32 ++++++++++++++-----
>  1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git
> a/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c
> b/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c
> index 93ff148..79a10c1 100644
> ---
> a/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c
> +++
> b/embeddedsw/ThirdParty/sw_services/lwip211/src/contrib/ports/xilinx/netif/xadapter.c
> @@ -126,8 +126,9 @@ xemac_add(struct netif *netif,
>         UINTPTR mac_baseaddr)
>  {
>         int i;
> +       struct netif * ret = NULL;
>
> -#if !NO_SYS
> +#if !NO_SYS && !defined(__rtems__)
>         /* Start thread to detect link periodically for Hot Plug
> autodetect */
>         sys_thread_new("link_detect_thread", link_detect_thread, netif,
>                         THREAD_STACKSIZE, tskIDLE_PRIORITY);
> @@ -142,7 +143,7 @@ xemac_add(struct netif *netif,
>                 switch (find_mac_type(mac_baseaddr)) {
>                         case xemac_type_xps_emaclite:
>  #ifdef XLWIP_CONFIG_INCLUDE_EMACLITE
> -                               return netif_add(netif, ipaddr, netmask,
> gw,
> +                               ret = netif_add(netif, ipaddr, netmask, gw,
>                                         (void*)mac_baseaddr,
>                                         xemacliteif_init,
>  #if NO_SYS
> @@ -151,12 +152,14 @@ xemac_add(struct netif *netif,
>                                         tcpip_input
>  #endif
>                                         );
> +                               break;
>  #else
> -                               return NULL;
> +                               ret = NULL;
> +                               break;
>  #endif
>                         case xemac_type_axi_ethernet:
>  #ifdef XLWIP_CONFIG_INCLUDE_AXI_ETHERNET
> -                               return netif_add(netif, ipaddr, netmask,
> gw,
> +                               ret = netif_add(netif, ipaddr, netmask, gw,
>                                         (void*)mac_baseaddr,
>                                         xaxiemacif_init,
>  #if NO_SYS
> @@ -165,16 +168,18 @@ xemac_add(struct netif *netif,
>                                         tcpip_input
>  #endif
>                                         );
> +                               break;
>  #else
> -                               return NULL;
> +                               ret = NULL;
> +                               break;
>  #endif
>  #if defined (__arm__) || defined (__aarch64__)
>                         case xemac_type_emacps:
>  #ifdef XLWIP_CONFIG_INCLUDE_GEM
>  #ifndef __rtems__
> -                               return netif_add(netif, ipaddr, netmask,
> gw,
> +                               ret = netif_add(netif, ipaddr, netmask, gw,
>  #else /* __rtems__ */
> -                               return netif_add( netif,
> +                               ret = netif_add( netif,
>                                                 (const ip4_addr_t *)
> ipaddr,
>                                                 (const ip4_addr_t *)
> netmask,
>                                                 (const ip4_addr_t *) gw,
> @@ -188,7 +193,10 @@ xemac_add(struct netif *netif,
>  #endif
>
>                                                 );
> +                               break;
>  #endif
> +                               ret = NULL;
> +                               break;
>  #endif
>                         default:
>  #ifndef __rtems__
> @@ -199,8 +207,16 @@ xemac_add(struct netif *netif,
>                                                 mac_baseaddr);
>                                 xil_printf("\r\n");
>  #endif
> -                               return NULL;
> +                               ret = NULL;
>         }
> +
> +#if defined(__rtems__)
> +       /* Start thread to detect link periodically for Hot Plug
> autodetect */
> +       sys_thread_new("link_detect_thread", link_detect_thread, netif,
> +                       THREAD_STACKSIZE, tskIDLE_PRIORITY);
> +#endif
> +
> +       return ret;
>  }
>
>  #if !NO_SYS
> --
> 2.34.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20240401/e77c9b59/attachment-0001.htm>