Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simulate FWP OS API failures in fwp_um.cpp #2405

Closed
shpalani opened this issue Apr 29, 2023 · 3 comments · Fixed by #2380 or #2403
Closed

Simulate FWP OS API failures in fwp_um.cpp #2405

shpalani opened this issue Apr 29, 2023 · 3 comments · Fixed by #2380 or #2403
Assignees
Labels
enhancement New feature or request triaged Discussed in a triage meeting
Milestone

Comments

@shpalani
Copy link
Collaborator

shpalani commented Apr 29, 2023

Describe the feature you'd like supported

Please refer to #2101. Creating this issue for the pending enhancement request to handle in 0.9.x

This issue is to add tests for the following, and fix the bugs coming out of it:

Add tests to simulate systematic failure of OS API failures for both ebpfcore and netebpfext.
This can use the same framework currently used for low-memory simulation in user mode.

OS APIs defined in fwp_um.cpp:

  1. FwpmTransactionCommit0
  2. FwpsFlowRemoveContext0
  3. FwpsFlowAssociateContext0
  4. FwpsAllocateNetBufferAndNetBufferList0
  5. FwpsInjectMacReceiveAsync0
  6. FwpsAllocateCloneNetBufferList0
  7. FwpsInjectMacSendAsync0
  8. FwpsAcquireWritableLayerDataPointer0
  9. FwpsAcquireClassifyHandle0
  10. FwpsRedirectHandleCreate0
  11. FwpsQueryConnectionRedirectState0

Proposed solution

Add fault injection in each of the FWP (Windows Filtering Platform) APIs defined in fwp_um.cpp.
The goal is to ensure:

  1. Drivers do not crash
  2. Drivers should not leak memory
  3. The callers of these APIs handle the returned failure cases.

Additional context

No response

@shpalani shpalani added the enhancement New feature or request label Apr 29, 2023
@shpalani shpalani self-assigned this Apr 29, 2023
@shpalani
Copy link
Collaborator Author

shpalani commented Apr 29, 2023

Adding FI to FwpmTransactionCommit0 gives memory leak, and crash.

Call Stack for crash and leak: In sock_addr, xdp

  1. Leak at net_ebpf_extension_xdp_on_client_attach.
Leak of 40 bytes at 2318349191312
    ebpf_allocate + 190 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\ebpf_platform_user.cpp:411)
    ExAllocatePoolUninitialized + 29 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\kernel_um.cpp:345)
    net_ebpf_extension_wfp_filter_context_create + 176 (C:\ebpf-2023\ebpf-for-windows\netebpfext\net_ebpf_ext.c:248)
    net_ebpf_extension_xdp_on_client_attach + 526 (C:\ebpf-2023\ebpf-for-windows\netebpfext\net_ebpf_ext_xdp.c:173)
    _net_ebpf_extension_hook_provider_attach_client + 613 (C:\ebpf-2023\ebpf-for-windows\netebpfext\net_ebpf_ext_hook_provider.c:362)
    _nmr::client_attach_provider + 553 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:139)
    NmrClientAttachProvider + 112 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_um.cpp:128)
    _netebpf_ext_helper::_hook_client_attach_provider + 258 (C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:205)
    `_nmr::bind'::`2'::<lambda_1>::operator() + 184 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:183)
    std::invoke<`_nmr::bind'::`2'::<lambda_1> &> + 20 (C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\type_traits:1565)
    std::_Invoker_ret<void>::_Call<`_nmr::bind'::`2'::<lambda_1> &> + 20 (C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\functional:674)
    std::_Func_impl_no_alloc<`_nmr::bind'::`2'::<lambda_1>,void>::_Do_call + 27 (C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\functional:834)
    std::_Func_class<void>::operator() + 80 (C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\functional:875)
    _nmr::perform_bind<std::map<void *,_nmr::client_registration,std::less<void *>,std::allocator<std::pair<void * const,_nmr::client_registration> > >,std::map<void *,_nmr::provider_registration,std::less<void *>,std::allocator<std::pair<void * const,_nmr::provider_registration> > > > + 758 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:353)
    _nmr::register_client + 96 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:48)
    NmrRegisterClient + 79 (C:\ebpf-2023\ebpf-for-windows\libs\platform\user\nmr_um.cpp:73)
    _netebpf_ext_helper::_nmr_client_registration::_nmr_client_registration + 401 (C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.h:131)
    std::make_unique<_netebpf_ext_helper::_nmr_client_registration,_NPI_CLIENT_CHARACTERISTICS *,_netebpfext_helper_base_client_context * &,0> + 77 (C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\memory:3393)
    _netebpf_ext_helper::_netebpf_ext_helper + 3433 (C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:79)
    CATCH2_INTERNAL_TEST_4 + 161 (C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpfext_unit.cpp:119)
    Catch::`anonymous namespace'::TestInvokerAsFunction::invoke + 18 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_test_registry.cpp:58)
    Catch::TestCaseHandle::invoke + 33 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\catch_test_case_info.hpp:116)
    Catch::RunContext::invokeActiveTestCase + 71 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:545)
    Catch::RunContext::runCurrentTest + 592 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:510)
    Catch::RunContext::runTest + 738 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:240)
    Catch::`anonymous namespace'::TestGroup::execute + 243 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:110)
    Catch::Session::runInternal + 1008 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:335)
    Catch::Session::run + 80 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:263)
    Catch::Session::run<char> + 82 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\catch_session.hpp:41)
    main + 97 (C:\ebpf-2023\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_main.cpp:36)
    invoke_main + 57 (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:79)
    __scrt_common_main_seh + 302 (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
    __scrt_common_main + 14 (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331)
Assertion failed: _allocations.empty(), file C:\ebpf-2023\ebpf-for-windows\libs\platform\user\ebpf_leak_detector.cpp, line 69

Analysis:
The leak is caused when Fault injection in FwpmFilterDeleteById0() is enabled, where the cleanup of inserted filters is not done during FwpmTransactionCommit0 failure.
Hence in UM, removed the fault injection in FwpmFilterDeleteById0.
In Kernel mode, FwpmFilterDeleteById0() will fail if invalid parameter is provided, so there is no cleanup of filter needed.

  1. Crash at ExAcquireRundownProtection
0:055> k
 # Child-SP          RetAddr               Call Site
00 0000006c`183fd660 00007ffa`89d73153     ucrtbased!_threadid+0x65
01 0000006c`183fd6b0 00007ffa`89d8ae2d     ucrtbased!_threadid+0x203
02 0000006c`183fd710 00007ffa`89d89db0     ucrtbased!abort+0x1d
03 0000006c`183fd750 00007ffa`de6e223a     ucrtbased!terminate+0x40
04 0000006c`183fd790 00007ffa`de6e2ec5     VCRUNTIME140_1D!_NLG_Return2+0x10ea
05 0000006c`183fd970 00007ffa`de6e2f57     VCRUNTIME140_1D!_NLG_Return2+0x1d75
06 0000006c`183fda40 00007ffa`de6e6ddb     VCRUNTIME140_1D!_NLG_Return2+0x1e07
07 0000006c`183fdaa0 00007ff6`457b8be0     VCRUNTIME140_1D!_CxxFrameHandler4+0xfb
08 0000006c`183fdb30 00007ffa`f1b53cff     netebpfext_unit!__GSHandlerCheck_EH4+0x90 [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\gs\amd64\gshandlereh4.cpp @ 73] 
09 0000006c`183fdb80 00007ffa`f1ace456     ntdll!_chkstk+0x12f
0a 0000006c`183fdbb0 00007ffa`f1b52cee     ntdll!RtlFindCharInUnicodeString+0xa96
0b 0000006c`183fe300 00007ffa`ef08fdec     ntdll!KiUserExceptionDispatcher+0x2e
0c 0000006c`183ff0c0 00007ffa`cab1b760     KERNELBASE!RaiseException+0x6c
0d 0000006c`183ff1a0 00007ff6`457c4ede     VCRUNTIME140D!CxxThrowException+0x120
0e 0000006c`183ff230 00007ff6`457c61c3     netebpfext_unit!_rundown_ref_table::acquire_rundown_ref+0x14e [C:\ebpf-2023\ebpf-for-windows\libs\platform\user\kernel_um.cpp @ 107] 
0f 0000006c`183ff360 00007ff6`457a4f35     netebpfext_unit!ExAcquireRundownProtection+0x23 [C:\ebpf-2023\ebpf-for-windows\libs\platform\user\kernel_um.cpp @ 254] 
10 0000006c`183ff3a0 00007ff6`4579bec5     netebpfext_unit!net_ebpf_extension_hook_client_enter_rundown+0x25 [C:\ebpf-2023\ebpf-for-windows\netebpfext\net_ebpf_ext_hook_provider.c @ 179] 
11 0000006c`183ff3e0 00007ff6`4578d07a     netebpfext_unit!net_ebpf_extension_sock_addr_authorize_recv_accept_classify+0xe5 [C:\ebpf-2023\ebpf-for-windows\netebpfext\net_ebpf_ext_sock_addr.c @ 1092] 
12 0000006c`183ff520 00007ff6`4578d857     netebpfext_unit!_fwp_engine::test_callout+0x29a [C:\ebpf-2023\ebpf-for-windows\netebpfext\user\fwp_um.cpp @ 149] 
13 0000006c`183ff7f0 00007ff6`45679998     netebpfext_unit!_fwp_engine::test_cgroup_inet4_recv_accept+0x177 [C:\ebpf-2023\ebpf-for-windows\netebpfext\user\fwp_um.cpp @ 174] 
14 0000006c`183ffad0 00007ff6`45677340     netebpfext_unit!_netebpf_ext_helper::test_cgroup_inet4_recv_accept+0x28 [C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.h @ 61] 
15 0000006c`183ffb10 00007ff6`45652128     netebpfext_unit!sock_addr_thread_function+0x130 [C:\ebpf-2023\ebpf-for-windows\tests\netebpfext_unit\netebpfext_unit.cpp @ 531] 
16 0000006c`183ffcb0 00007ff6`4564a5c6     netebpfext_unit!std::invoke<void (__cdecl*)(std::stop_token,_netebpf_ext_helper *,_fwp_classify_parameters *,enum _sock_addr_test_type,unsigned short,unsigned short),std::stop_token,_netebpf_ext_helper *,_fwp_classify_parameters *,enum _sock_addr_test_type,unsigned short,unsigned short>+0xe8 [C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\type_traits @ 1589] 
17 0000006c`183ffd50 00007ffa`89d93010     netebpfext_unit!std::thread::_Invoke<std::tuple<void (__cdecl*)(std::stop_token,_netebpf_ext_helper *,_fwp_classify_parameters *,enum _sock_addr_test_type,unsigned short,unsigned short),std::stop_token,_netebpf_ext_helper *,_fwp_classify_parameters *,enum _sock_addr_test_type,unsigned short,unsigned short>,0,1,2,3,4,5,6>+0x126 [C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\include\thread @ 56] 
18 0000006c`183ffe10 00007ffa`f091269d     ucrtbased!register_onexit_function+0x130
19 0000006c`183ffe70 00007ffa`f1b0a9f8     KERNEL32!BaseThreadInitThunk+0x1d
1a 0000006c`183ffea0 00000000`00000000     ntdll!RtlUserThreadStart+0x28

@shpalani
Copy link
Collaborator Author

shpalani commented Apr 29, 2023

RCA:

==5164==ERROR: AddressSanitizer: heap-use-after-free on address 0x12c85fb22698 at pc 0x7ff7ca36f24a bp 0x00fc7ef8a530 sp 0x00fc7ef8a538
READ of size 8 at 0x12c85fb22698 thread T0
    #0 0x7ff7ca36f249 in net_ebpf_extension_sock_addr_authorize_recv_accept_classify(struct FWPS_INCOMING_VALUES0_ const *, struct FWPS_INCOMING_METADATA_VALUES0_ const *, void *, void const *, struct FWPS_FILTER3_ const *, unsigned __int64, struct FWPS_CLASSIFY_OUT0_*) D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext_sock_addr.c:1087
    #1 0x7ff7ca34eea7 in _fwp_engine::test_callout(unsigned short, struct _GUID const &, struct _GUID const &, struct FWPS_INCOMING_VALUE0_*) D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\user\fwp_um.cpp:140
    #2 0x7ff7ca3504d5 in _fwp_engine::test_cgroup_inet4_recv_accept(struct _fwp_classify_parameters *) D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\user\fwp_um.cpp:172
    #3 0x7ff7ca0baf66 in _netebpf_ext_helper::test_cgroup_inet4_recv_accept(struct _fwp_classify_parameters *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.h:60
    #4 0x7ff7ca07f2a8 in CATCH2_INTERNAL_TEST_12 D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpfext_unit.cpp:427
    #5 0x7ff7ca0e6e80 in Catch::`anonymous namespace'::TestInvokerAsFunction::invoke D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_test_registry.cpp:58
    #6 0x7ff7ca112b2f in Catch::TestCaseHandle::invoke(void) const D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_test_case_info.hpp:115
    #7 0x7ff7ca112c51 in Catch::RunContext::invokeActiveTestCase(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:545
    #8 0x7ff7ca115a57 in Catch::RunContext::runCurrentTest(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:508
    #9 0x7ff7ca116783 in Catch::RunContext::runTest(class Catch::TestCaseHandle const &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:239
    #10 0x7ff7ca3cff41 in Catch::`anonymous namespace'::TestGroup::execute D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:110
    #11 0x7ff7ca3d2db3 in Catch::Session::runInternal(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:332
    #12 0x7ff7ca3d2178 in Catch::Session::run(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:263
    #13 0x7ff7ca3985d6 in Catch::Session::run<char>(int, char const *const *const) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.hpp:41
    #14 0x7ff7ca398700 in main D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_main.cpp:36
    #15 0x7ff7ca394038 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #16 0x7ff7ca393f8d in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #17 0x7ff7ca393e4d in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #18 0x7ff7ca3940ad in mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:16
    #19 0x7ffaf091269c  (C:\Windows\System32\KERNEL32.DLL+0x18001269c)
    #20 0x7ffaf1b0a9f7  (C:\Windows\SYSTEM32\ntdll.dll+0x18005a9f7)

0x12c85fb22698 is located 8 bytes inside of 40-byte region [0x12c85fb22690,0x12c85fb226b8)
freed by thread T0 here:
    #0 0x7ffa7df328b4  (C:\Users\shpalan\Downloads\Build-x64-Sanitize-Debug\build-Debug\Debug\clang_rt.asan_dbg_dynamic-x86_64.dll+0x1800528b4)
    #1 0x7ff7ca2c4496 in ebpf_free D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\ebpf_platform_user.cpp:461
    #2 0x7ff7ca3aee32 in ExFreePool D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\kernel_um.cpp:350
    #3 0x7ff7ca36a082 in _net_ebpf_extension_sock_addr_on_client_attach D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext_sock_addr.c:389
    #4 0x7ff7ca38060e in _net_ebpf_extension_hook_provider_attach_client D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext_hook_provider.c:362
    #5 0x7ff7ca31fd31 in _nmr::client_attach_provider(void *, void const *, void const *, void const **, void const **) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:139
    #6 0x7ff7ca2e67be in NmrClientAttachProvider D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_um.cpp:128
    #7 0x7ff7ca0cb292 in _netebpf_ext_helper::_hook_client_attach_provider(void *, void *, struct _NPI_REGISTRATION_INSTANCE const *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:195
    #8 0x7ff7ca30ffff in `_nmr::bind'::`2'::<lambda_1>::operator() D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:183
    #9 0x7ff7ca306bd2 in std::invoke<`_nmr::bind'::`2'::<lambda_1> &> C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\type_traits:1562
    #10 0x7ff7ca2fc7d2 in std::_Invoker_ret<void>::_Call<`_nmr::bind'::`2'::<lambda_1> &> C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:670
    #11 0x7ff7ca311da9 in std::_Func_impl_no_alloc<`_nmr::bind'::`2'::<lambda_1>,void>::_Do_call C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:833
    #12 0x7ff7ca310594 in std::_Func_class<void>::operator()(void) const C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:874
    #13 0x7ff7ca3072f1 in _nmr::perform_bind<class std::map<void *, struct _nmr::client_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::client_registration>>>, class std::map<void *, struct _nmr::provider_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::provider_registration>>>>(class std::map<void *, struct _nmr::client_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::client_registration>>> &, void *const, class std::map<void *, struct _nmr::provider_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::provider_registration>>> &) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:352
    #14 0x7ff7ca3214de in _nmr::register_client(struct _NPI_CLIENT_CHARACTERISTICS const &, void const *) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:47
    #15 0x7ff7ca2e697c in NmrRegisterClient D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_um.cpp:73
    #16 0x7ff7ca0c6304 in _netebpf_ext_helper::_nmr_client_registration::_nmr_client_registration(struct _NPI_CLIENT_CHARACTERISTICS const *, void *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.h:131
    #17 0x7ff7ca0c2aa6 in std::make_unique<struct _netebpf_ext_helper::_nmr_client_registration, struct _NPI_CLIENT_CHARACTERISTICS *, struct _netebpfext_helper_base_client_context *&, 0>(struct _NPI_CLIENT_CHARACTERISTICS *&&, struct _netebpfext_helper_base_client_context *&) C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\memory:3433
    #18 0x7ff7ca0c5cd3 in _netebpf_ext_helper::_netebpf_ext_helper(void const *, enum ebpf_result (__cdecl *)(void), struct _netebpfext_helper_base_client_context *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:76
    #19 0x7ff7ca07f1c4 in CATCH2_INTERNAL_TEST_12 D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpfext_unit.cpp:416
    #20 0x7ff7ca0e6e80 in Catch::`anonymous namespace'::TestInvokerAsFunction::invoke D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_test_registry.cpp:58
    #21 0x7ff7ca112b2f in Catch::TestCaseHandle::invoke(void) const D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_test_case_info.hpp:115
    #22 0x7ff7ca112c51 in Catch::RunContext::invokeActiveTestCase(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:545
    #23 0x7ff7ca115a57 in Catch::RunContext::runCurrentTest(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:508
    #24 0x7ff7ca116783 in Catch::RunContext::runTest(class Catch::TestCaseHandle const &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:239
    #25 0x7ff7ca3cff41 in Catch::`anonymous namespace'::TestGroup::execute D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:110
    #26 0x7ff7ca3d2db3 in Catch::Session::runInternal(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:332
    #27 0x7ff7ca3d2178 in Catch::Session::run(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:263
    #28 0x7ff7ca3985d6 in Catch::Session::run<char>(int, char const *const *const) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.hpp:41

previously allocated by thread T0 here:
    #0 0x7ffa7df32716  (C:\Users\shpalan\Downloads\Build-x64-Sanitize-Debug\build-Debug\Debug\clang_rt.asan_dbg_dynamic-x86_64.dll+0x180052716)
    #1 0x7ff7ca2c20e1 in ebpf_allocate D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\ebpf_platform_user.cpp:402
    #2 0x7ff7ca3aee0b in ExAllocatePoolUninitialized D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\kernel_um.cpp:344
    #3 0x7ff7ca35a384 in net_ebpf_extension_wfp_filter_context_create(unsigned __int64, struct _net_ebpf_extension_hook_client const *, struct _net_ebpf_extension_wfp_filter_context **) D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext.c:248
    #4 0x7ff7ca369a60 in _net_ebpf_extension_sock_addr_on_client_attach D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext_sock_addr.c:350
    #5 0x7ff7ca38060e in _net_ebpf_extension_hook_provider_attach_client D:\a\ebpf-for-windows\ebpf-for-windows\netebpfext\net_ebpf_ext_hook_provider.c:362
    #6 0x7ff7ca31fd31 in _nmr::client_attach_provider(void *, void const *, void const *, void const **, void const **) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:139
    #7 0x7ff7ca2e67be in NmrClientAttachProvider D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_um.cpp:128
    #8 0x7ff7ca0cb292 in _netebpf_ext_helper::_hook_client_attach_provider(void *, void *, struct _NPI_REGISTRATION_INSTANCE const *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:195
    #9 0x7ff7ca30ffff in `_nmr::bind'::`2'::<lambda_1>::operator() D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:183
    #10 0x7ff7ca306bd2 in std::invoke<`_nmr::bind'::`2'::<lambda_1> &> C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\type_traits:1562
    #11 0x7ff7ca2fc7d2 in std::_Invoker_ret<void>::_Call<`_nmr::bind'::`2'::<lambda_1> &> C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:670
    #12 0x7ff7ca311da9 in std::_Func_impl_no_alloc<`_nmr::bind'::`2'::<lambda_1>,void>::_Do_call C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:833
    #13 0x7ff7ca310594 in std::_Func_class<void>::operator()(void) const C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\functional:874
    #14 0x7ff7ca3072f1 in _nmr::perform_bind<class std::map<void *, struct _nmr::client_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::client_registration>>>, class std::map<void *, struct _nmr::provider_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::provider_registration>>>>(class std::map<void *, struct _nmr::client_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::client_registration>>> &, void *const, class std::map<void *, struct _nmr::provider_registration, struct std::less<void *>, class std::allocator<struct std::pair<void *const, struct _nmr::provider_registration>>> &) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:352
    #15 0x7ff7ca3214de in _nmr::register_client(struct _NPI_CLIENT_CHARACTERISTICS const &, void const *) D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_impl.cpp:47
    #16 0x7ff7ca2e697c in NmrRegisterClient D:\a\ebpf-for-windows\ebpf-for-windows\libs\platform\user\nmr_um.cpp:73
    #17 0x7ff7ca0c6304 in _netebpf_ext_helper::_nmr_client_registration::_nmr_client_registration(struct _NPI_CLIENT_CHARACTERISTICS const *, void *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.h:131
    #18 0x7ff7ca0c2aa6 in std::make_unique<struct _netebpf_ext_helper::_nmr_client_registration, struct _NPI_CLIENT_CHARACTERISTICS *, struct _netebpfext_helper_base_client_context *&, 0>(struct _NPI_CLIENT_CHARACTERISTICS *&&, struct _netebpfext_helper_base_client_context *&) C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\memory:3433
    #19 0x7ff7ca0c5cd3 in _netebpf_ext_helper::_netebpf_ext_helper(void const *, enum ebpf_result (__cdecl *)(void), struct _netebpfext_helper_base_client_context *) D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpf_ext_helper.cpp:76
    #20 0x7ff7ca07f1c4 in CATCH2_INTERNAL_TEST_12 D:\a\ebpf-for-windows\ebpf-for-windows\tests\netebpfext_unit\netebpfext_unit.cpp:416
    #21 0x7ff7ca0e6e80 in Catch::`anonymous namespace'::TestInvokerAsFunction::invoke D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_test_registry.cpp:58
    #22 0x7ff7ca112b2f in Catch::TestCaseHandle::invoke(void) const D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_test_case_info.hpp:115
    #23 0x7ff7ca112c51 in Catch::RunContext::invokeActiveTestCase(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:545
    #24 0x7ff7ca115a57 in Catch::RunContext::runCurrentTest(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:508
    #25 0x7ff7ca116783 in Catch::RunContext::runTest(class Catch::TestCaseHandle const &) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_run_context.cpp:239
    #26 0x7ff7ca3cff41 in Catch::`anonymous namespace'::TestGroup::execute D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:110
    #27 0x7ff7ca3d2db3 in Catch::Session::runInternal(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:332
    #28 0x7ff7ca3d2178 in Catch::Session::run(void) D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\catch_session.cpp:263

Analysis:

  • Close as 'Not fix'.

Reason:

  • In Kernel mode, as per the WFP experts, if FwpmTransactionCommit0 fails, it is None or All. WFP does the cleanup for the caller. So, the Kernel mode should be fine in freeing the filter_context, and the hook client associated with the filter_context will not be accessed.

  • In the User mode netebpf_ext_helper_t code, if FwpmTransactionCommit0 fails in _net_ebpf_extension_sock_addr_on_client_attach(), there are callout/classify callbacks in sock_addr_invoke test case which requires filter context, and acquires Runtime protection that is not initialized. Hence the cleanup is not happening for the added/inserted filters via FwpmFilterAdd0 but filter_context is freed, thereby causing to heap-use-after-free of the filter_context.

    sock_addr_invoke
     | --  netebpf_ext_helper_t helper    <<< FwpmTransactionCommit0 fails here.
     | -- helper.test_cgroup_inet4_recv_accept
     | -- helper.test_cgroup_inet6_recv_accept
     | -- helper.test_cgroup_inet4_connect
     | -- helper.test_cgroup_inet6_connect
     | -- ...

@shankarseal
Copy link
Collaborator

Do not simulate failure on APIs that can only fail on incorrect parameters - or any API that is used to free resources. Rest are OK to simulate failure, on a case by case basis.

@shankarseal shankarseal added the triaged Discussed in a triage meeting label May 1, 2023
@shankarseal shankarseal added this to the 2305 milestone May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged Discussed in a triage meeting
Projects
None yet
2 participants