RE: [PATCH v2] fix eal/linux: unregister alarm callback before free

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> This was flagged by Address sanitizer as a use after free. The
> intr_handle ptr is shared between the main thread and the interrupt
> thread. The interrupt thread can dereference the ptr after free (from
> the alarm callback). free is called when the main thread cleans up.
> 
> The interrupt thread never terminates (eal_intr_thread_main) so
> use rte_intr_callback_unregister_sync during cleanup to
> ensure the callback is removed before freeing the ptr.
> 
> To be more defensive clear out the pointer and registration
> variable if we can unregister.
> 
> rte_intr_callback_unregister_sync may (optionally) use traces
> so the alarm cleanup must happen before eal_trace_fini to avoid
> accessing freed memory.
> 
> Bugzilla ID: 1683
> 
> Signed-off-by: Rui Ferreira <rui.ferreira1@h-partners.com>
> ---
>  .mailmap                  | 1 +
>  lib/eal/linux/eal.c       | 3 ++-
>  lib/eal/linux/eal_alarm.c | 9 ++++++++-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index d8439b79ce..907c5ea967 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1332,6 +1332,7 @@ Rosen Xu <rosen.xu@altera.com> <rosen.xu@intel.com>
>  Roy Franz <roy.franz@cavium.com>
>  Roy Pledge <roy.pledge@nxp.com>
>  Roy Shterman <roy.shterman@vastdata.com>
> +Rui Ferreira <rui.ferreira1@h-partners.com>
>  Ruifeng Wang <ruifeng.wang@arm.com>
>  Rushil Gupta <rushilg@google.com>
>  Ryan E Hall <ryan.e.hall@intel.com>
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 20f777b8b0..b448db1392 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -1329,9 +1329,10 @@ rte_eal_cleanup(void)
>  	rte_mp_channel_cleanup();
>  	eal_bus_cleanup();
>  	rte_trace_save();
> +	/* may use trace, must be called before eal_trace_fini */
> +	rte_eal_alarm_cleanup();
>  	eal_trace_fini();
>  	eal_mp_dev_hotplug_cleanup();
> -	rte_eal_alarm_cleanup();
>  	/* after this point, any DPDK pointers will become dangling */
>  	rte_eal_memory_detach();
>  	rte_eal_malloc_heap_cleanup();
> diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
> index b216a007a3..eb6a21d4f0 100644
> --- a/lib/eal/linux/eal_alarm.c
> +++ b/lib/eal/linux/eal_alarm.c
> @@ -57,7 +57,14 @@ static void eal_alarm_callback(void *arg);
>  void
>  rte_eal_alarm_cleanup(void)
>  {
> -	rte_intr_instance_free(intr_handle);
> +	/* unregister callback using intr_handle in interrupt thread */
> +	int ret = rte_intr_callback_unregister_sync(intr_handle,
> +						eal_alarm_callback, (void *)-1);
> +	if (ret >= 0) {
> +		rte_intr_instance_free(intr_handle);
> +		intr_handle = NULL;
> +		handler_registered = 0;
> +	}
>  }
> 
>  int
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
 

> 2.35.3