[PATCH] eal/linux: unregister alarm callback before free ptr

[PATCH] eal/linux: unregister alarm callback before free ptr

[Rui Ferreira]

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, and the interrupt thread can dereference the ptr after free
is called when the main thread cleans up (from the alarm callback).

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.

Bugzilla ID: 1683

Signed-off-by: Rui Ferreira <rui.ferreira1@h-partners.com>
---
 .mailmap                  | 1 +
 lib/eal/linux/eal_alarm.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

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_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
-- 
2.35.3

Re: [PATCH] eal/linux: unregister alarm callback before free ptr

[David Marchand]

On Tue, May 20, 2025 at 5:08 PM Rui Ferreira
<rui.ferreira1@h-partners.com> wrote:
>
> 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, and the interrupt thread can dereference the ptr after free
> is called when the main thread cleans up (from the alarm callback).
>
> 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.
>
> Bugzilla ID: 1683
>
> Signed-off-by: Rui Ferreira <rui.ferreira1@h-partners.com>

I remember mentioning that other OS may be affected by the bug.
Please consider fixing this issue for Windows and FreeBSD too.


> ---
>  .mailmap                  | 1 +
>  lib/eal/linux/eal_alarm.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> 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_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

In rte_eal_cleanup, the trace framework is uninitialised prior to
rte_eal_alarm_cleanup().

And the CI caught this issue (see ovsrobot report in patchwork).

ERROR: AddressSanitizer: heap-use-after-free on address 0x7f872ca90f80
at pc 0x7f873db9fb1e bp 0x7fffc98a3720 sp 0x7fffc98a3718
READ of size 4 at 0x7f872ca90f80 thread T0
    #0 0x7f873db9fb1d in __rte_trace_mem_get
/home/runner/work/dpdk/dpdk/build/../lib/eal/include/rte_trace_point.h:331:27
    #1 0x7f873db9fb1d in rte_eal_trace_intr_callback_unregister
/home/runner/work/dpdk/dpdk/build/../lib/eal/include/eal_trace_internal.h:58:1
    #2 0x7f873db9fb1d in rte_intr_callback_unregister
/home/runner/work/dpdk/dpdk/build/../lib/eal/linux/eal_interrupts.c:685:2
    #3 0x7f873db9fe44 in rte_intr_callback_unregister_sync
/home/runner/work/dpdk/dpdk/build/../lib/eal/linux/eal_interrupts.c:697:16
    #4 0x7f873db996ad in rte_eal_alarm_cleanup
/home/runner/work/dpdk/dpdk/build/../lib/eal/linux/eal_alarm.c:61:12
    #5 0x7f873db98b4f in rte_eal_cleanup
/home/runner/work/dpdk/dpdk/build/../lib/eal/linux/eal.c:1334:2
    #6 0x561c211092db in main
/home/runner/work/dpdk/dpdk/build/../app/test/test.c:263:2
    #7 0x7f873c429d8f in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #8 0x7f873c429e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #9 0x561c2104a534 in _start
(/home/runner/work/dpdk/dpdk/build/app/dpdk-test+0x1fc534) (BuildId:
37b8659fbf174b3d78222edbdcd9f2f6a3027aff)

Address 0x7f872ca90f80 is a wild pointer inside of access range of
size 0x000000000004.
SUMMARY: AddressSanitizer: heap-use-after-free
/home/runner/work/dpdk/dpdk/build/../lib/eal/include/rte_trace_point.h:331:27
in __rte_trace_mem_get
Shadow bytes around the buggy address:
  0x0ff16594a1a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a1b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a1c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a1d0: fd fd fd fd fd fd fd fd 00 00 00 00 00 00 00 00
  0x0ff16594a1e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ff16594a1f0:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a210: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a220: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a230: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff16594a240: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==55106==ABORTING


-- 
David Marchand

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

[Konstantin Ananyev]

> Subject: [PATCH] eal/linux: unregister alarm callback before free ptr
> 
> 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, and the interrupt thread can dereference the ptr after free
> is called when the main thread cleans up (from the alarm callback).
> 
> 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.
> 
> Bugzilla ID: 1683
> 
> Signed-off-by: Rui Ferreira <rui.ferreira1@h-partners.com>
> ---
>  .mailmap                  | 1 +
>  lib/eal/linux/eal_alarm.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> 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_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>

As a nit: as it is a bug-fix, probably start with 'fix ..' in the subject.
Also "Fixes:" and probably " Cc: stable@dpdk.org" needs to be added.

> 2.35.3

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

[Rui Ferreira]

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
-- 
2.35.3

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

[Konstantin Ananyev]

> 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

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

[Thomas Monjalon]

Hello,

Did you see the comment from David about fixing for Windows and FreeBSD at the same time?
I think you just need to apply the same logic in 3 different places.
If you cannot test all, that's OK to submit the changes without full coverage testing.

Thank you


22/05/2025 17:59, Rui Ferreira:
> 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(-)

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

[Rui Ferreira (A)]

Hi. I'll try to have a look in the next couple days. 

BR

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, May 27, 2025 3:54 PM
> To: Rui Ferreira (A) <rui.ferreira1@h-partners.com>
> Cc: dev@dpdk.org; stable@dpdk.org; David Marchand
> <david.marchand@redhat.com>; Konstantin Ananyev
> <konstantin.ananyev@huawei.com>
> Subject: Re: [PATCH v2] fix eal/linux: unregister alarm callback before free
> 
> Hello,
> 
> Did you see the comment from David about fixing for Windows and FreeBSD
> at the same time?
> I think you just need to apply the same logic in 3 different places.
> If you cannot test all, that's OK to submit the changes without full coverage
> testing.
> 
> Thank you
> 
> 
> 22/05/2025 17:59, Rui Ferreira:
> > 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(-)
> 
> 

[PATCH v2 0/2] fix eal: unregister alarm callback before free

[Rui Ferreira]

This was flagged by Address sanitizer as a use after free in Linux. A
pointer is freed in the main thread but used by a callback in another thread.
It can be reliably reproduced in Linux with ASAN but I could not trigger the
same issue in FreeBSD.

I've included nearly identical patches for Linux and FreeBSD. I did not
include any changes for Windows since the same issue does not seem to apply
there (thread is actually cancelled, no shared ptr).

Bugzilla ID: 1683

Rui Ferreira (2):
  fix eal/linux: unregister alarm callback before free
  fix eal/freebsd: unregister alarm callback before free

 .mailmap                    | 1 +
 lib/eal/freebsd/eal.c       | 3 ++-
 lib/eal/freebsd/eal_alarm.c | 8 +++++++-
 lib/eal/linux/eal.c         | 3 ++-
 lib/eal/linux/eal_alarm.c   | 9 ++++++++-
 5 files changed, 20 insertions(+), 4 deletions(-)

--
2.43.0

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

[Rui Ferreira]

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
-- 
2.43.0

[PATCH v2 2/2] fix eal/freebsd: unregister alarm callback before free

[Rui Ferreira]

Unregister callback on cleanup to avoid use after free from the
interrupt thread (eal_intr_thread_main).

To be more defensive, set ptr to NULL 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>
---
 lib/eal/freebsd/eal.c       | 3 ++-
 lib/eal/freebsd/eal_alarm.c | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index d6fffa2170..21ce4e6b18 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -907,8 +907,9 @@ rte_eal_cleanup(void)
 	rte_mp_channel_cleanup();
 	eal_bus_cleanup();
 	rte_trace_save();
-	eal_trace_fini();
+	/* may use trace, must be called before eal_trace_fini */
 	rte_eal_alarm_cleanup();
+	eal_trace_fini();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	eal_cleanup_config(internal_conf);
diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c
index 28f285fdef..c03e281e67 100644
--- a/lib/eal/freebsd/eal_alarm.c
+++ b/lib/eal/freebsd/eal_alarm.c
@@ -50,7 +50,13 @@ 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;
+	}
 }
 
 int
-- 
2.43.0