mbox series

[v2,0/2] fix race in rte_thread_create failure path

Message ID 1678750267-3829-1-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
Headers
Series fix race in rte_thread_create failure path |

Message

Tyler Retzlaff March 13, 2023, 11:31 p.m. UTC 
  v2:
  * new approach over v1 of the patch to avoid using pthread np API that
    is not available on Alpine Linux.
  * to conform to rte_thread_create parameter const qualification include
    an additional patch to const qualify rte_thread_set_affinity cpusetp
    parameter.

Tyler Retzlaff (2):
  eal: make cpusetp to rte thread set affinity const
  eal: fix failure path race setting new thread affinity

 lib/eal/common/eal_common_thread.c |  6 ++---
 lib/eal/include/rte_thread.h       |  2 +-
 lib/eal/unix/rte_thread.c          | 52 ++++++++++++++++++++++++++++++--------
 3 files changed, 46 insertions(+), 14 deletions(-)

Comments

David Marchand March 14, 2023, 11:47 a.m. UTC | #1 
On Tue, Mar 14, 2023 at 12:31 AM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> v2:
>   * new approach over v1 of the patch to avoid using pthread np API that
>     is not available on Alpine Linux.
>   * to conform to rte_thread_create parameter const qualification include
>     an additional patch to const qualify rte_thread_set_affinity cpusetp
>     parameter.
>
> Tyler Retzlaff (2):
>   eal: make cpusetp to rte thread set affinity const
>   eal: fix failure path race setting new thread affinity
>
>  lib/eal/common/eal_common_thread.c |  6 ++---
>  lib/eal/include/rte_thread.h       |  2 +-
>  lib/eal/unix/rte_thread.c          | 52 ++++++++++++++++++++++++++++++--------
>  3 files changed, 46 insertions(+), 14 deletions(-)

ASan flagged some use after free.
See logs https://github.com/ovsrobot/dpdk/suites/11537702259/artifacts/597032673

24/90 DPDK:fast-tests / lcores_autotest       FAIL     1.72 s (exit status 1)

--- command ---
00:24:14 DPDK_TEST='lcores_autotest'
/home/runner/work/dpdk/dpdk/build/app/test/dpdk-test
--file-prefix=lcores_autotest
--- stdout ---
RTE>>lcores_autotest
--- stderr ---
EAL: Detected CPU lcores: 2
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/lcores_autotest/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: VFIO support initialized
APP: HPET is not enabled, using TSC as default timer
=================================================================
==70246==ERROR: AddressSanitizer: heap-use-after-free on address
0x60300000d044 at pc 0x7f6c9c49e1cf bp 0x7ffdbf1b3670 sp
0x7ffdbf1b3668
READ of size 4 at 0x60300000d044 thread T0
    #0 0x7f6c9c49e1ce in rte_thread_create
/home/runner/work/dpdk/dpdk/build/../lib/eal/unix/rte_thread.c:196:3
    #1 0x957e16 in test_non_eal_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:81:7
    #2 0x957e16 in test_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:400:6
    #3 0x4dcbc0 in cmd_autotest_parsed
/home/runner/work/dpdk/dpdk/build/../app/test/commands.c:68:10
    #4 0x7f6c9c0d3a88 in __cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:294:3
    #5 0x7f6c9c0d3a88 in cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:302:9
    #6 0x7f6c9c0d0907 in cmdline_valid_buffer
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:24:8
    #7 0x7f6c9c0d91c4 in rdline_char_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_rdline.c:444:5
    #8 0x7f6c9c0d0cd8 in cmdline_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:146:9
    #9 0x510205 in main
/home/runner/work/dpdk/dpdk/build/../app/test/test.c:208:15
    #10 0x7f6c9a92d082 in __libc_start_main
/build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #11 0x432e4d in _start
(/home/runner/work/dpdk/dpdk/build/app/test/dpdk-test+0x432e4d)

0x60300000d044 is located 20 bytes inside of 32-byte region
[0x60300000d030,0x60300000d050)
freed by thread T6 here:
    #0 0x4acc3d in free
(/home/runner/work/dpdk/dpdk/build/app/test/dpdk-test+0x4acc3d)
    #1 0x7f6c9c49de64 in thread_func_wrapper
/home/runner/work/dpdk/dpdk/build/../lib/eal/unix/rte_thread.c:111:2
    #2 0x7f6c9ab28608 in start_thread
/build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477:8

previously allocated by thread T0 here:
    #0 0x4ad032 in calloc
(/home/runner/work/dpdk/dpdk/build/app/test/dpdk-test+0x4ad032)
    #1 0x7f6c9c49e021 in rte_thread_create
/home/runner/work/dpdk/dpdk/build/../lib/eal/unix/rte_thread.c:131:8
    #2 0x957e16 in test_non_eal_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:81:7
    #3 0x957e16 in test_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:400:6
    #4 0x4dcbc0 in cmd_autotest_parsed
/home/runner/work/dpdk/dpdk/build/../app/test/commands.c:68:10
    #5 0x7f6c9c0d3a88 in __cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:294:3
    #6 0x7f6c9c0d3a88 in cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:302:9
    #7 0x7f6c9c0d0907 in cmdline_valid_buffer
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:24:8
    #8 0x7f6c9c0d91c4 in rdline_char_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_rdline.c:444:5
    #9 0x7f6c9c0d0cd8 in cmdline_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:146:9
    #10 0x510205 in main
/home/runner/work/dpdk/dpdk/build/../app/test/test.c:208:15
    #11 0x7f6c9a92d082 in __libc_start_main
/build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

Thread T6 created by T0 here:
    #0 0x4978ea in pthread_create
(/home/runner/work/dpdk/dpdk/build/app/test/dpdk-test+0x4978ea)
    #1 0x7f6c9c49e117 in rte_thread_create
/home/runner/work/dpdk/dpdk/build/../lib/eal/unix/rte_thread.c:187:8
    #2 0x957e16 in test_non_eal_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:81:7
    #3 0x957e16 in test_lcores
/home/runner/work/dpdk/dpdk/build/../app/test/test_lcores.c:400:6
    #4 0x4dcbc0 in cmd_autotest_parsed
/home/runner/work/dpdk/dpdk/build/../app/test/commands.c:68:10
    #5 0x7f6c9c0d3a88 in __cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:294:3
    #6 0x7f6c9c0d3a88 in cmdline_parse
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_parse.c:302:9
    #7 0x7f6c9c0d0907 in cmdline_valid_buffer
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:24:8
    #8 0x7f6c9c0d91c4 in rdline_char_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline_rdline.c:444:5
    #9 0x7f6c9c0d0cd8 in cmdline_in
/home/runner/work/dpdk/dpdk/build/../lib/cmdline/cmdline.c:146:9
    #10 0x510205 in main
/home/runner/work/dpdk/dpdk/build/../app/test/test.c:208:15
    #11 0x7f6c9a92d082 in __libc_start_main
/build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-use-after-free
/home/runner/work/dpdk/dpdk/build/../lib/eal/unix/rte_thread.c:196:3
in rte_thread_create
Shadow bytes around the buggy address:
  0x0c067fff99b0: fa fa 00 00 01 fa fa fa 00 00 00 00 fa fa 00 00
  0x0c067fff99c0: 00 00 fa fa 00 00 00 fa fa fa 00 00 00 06 fa fa
  0x0c067fff99d0: fd fd fd fa fa fa fd fd fd fa fa fa 00 00 00 07
  0x0c067fff99e0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
  0x0c067fff99f0: fd fd fa fa 00 00 00 07 fa fa 00 00 01 fa fa fa
=>0x0c067fff9a00: 00 00 04 fa fa fa fd fd[fd]fd fa fa fa fa fa fa
  0x0c067fff9a10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9a20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9a30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9a40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9a50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Shadow gap:              cc
==70246==ABORTING
-------
Tyler Retzlaff March 14, 2023, 1:59 p.m. UTC | #2 
On Tue, Mar 14, 2023 at 12:47:54PM +0100, David Marchand wrote:
> On Tue, Mar 14, 2023 at 12:31 AM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > v2:
> >   * new approach over v1 of the patch to avoid using pthread np API that
> >     is not available on Alpine Linux.
> >   * to conform to rte_thread_create parameter const qualification include
> >     an additional patch to const qualify rte_thread_set_affinity cpusetp
> >     parameter.
> >
> > Tyler Retzlaff (2):
> >   eal: make cpusetp to rte thread set affinity const
> >   eal: fix failure path race setting new thread affinity
> >
> >  lib/eal/common/eal_common_thread.c |  6 ++---
> >  lib/eal/include/rte_thread.h       |  2 +-
> >  lib/eal/unix/rte_thread.c          | 52 ++++++++++++++++++++++++++++++--------
> >  3 files changed, 46 insertions(+), 14 deletions(-)
> 
> ASan flagged some use after free.
> See logs https://github.com/ovsrobot/dpdk/suites/11537702259/artifacts/597032673
> 
> 24/90 DPDK:fast-tests / lcores_autotest       FAIL     1.72 s (exit status 1)

thanks, i'm waiting on a system to test plan to investigate today if it
becomes available.

ty