On Tue, May 20, 2025 at 5:08 PM Rui Ferreira
<rui.ferrei...@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.ferrei...@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...@altera.com> <rosen...@intel.com>
> Roy Franz <roy.fr...@cavium.com>
> Roy Pledge <roy.ple...@nxp.com>
> Roy Shterman <roy.shter...@vastdata.com>
> +Rui Ferreira <rui.ferrei...@h-partners.com>
> Ruifeng Wang <ruifeng.w...@arm.com>
> Rushil Gupta <rush...@google.com>
> Ryan E Hall <ryan.e.h...@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
