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.

Sign up for GitHub

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

(DRAFT PR - NOT FOR REVIEW) mt stress fail fix (WFP transaction issue) #3685

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

(DRAFT PR - NOT FOR REVIEW) mt stress fail fix (WFP transaction issue) #3685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b08d60
Use static WFP session for provider addition, dynamic for each filter…
Jun 20, 2024
eb94306
Use a dynamic global WFP session with per-filter-context static sessions
Jul 15, 2024
eabfc73
Merge branch 'microsoft:main' into 3602-mt-stress-fail-fix
dv-msft Aug 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 47 additions & 36 deletions netebpfext/net_ebpf_ext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ static net_ebpf_ext_wfp_callout_state_t _net_ebpf_ext_wfp_callout_states[] = {
}};

// WFP globals
static HANDLE _fwp_engine_handle;
static HANDLE _wfp_engine_handle;

//
// WFP component management related utility functions.
Expand All @@ -237,8 +237,10 @@ net_ebpf_extension_wfp_filter_context_create(
_In_ const net_ebpf_extension_hook_client_t* client_context,
_Outptr_ net_ebpf_extension_wfp_filter_context_t** filter_context)
{
NTSTATUS status = STATUS_SUCCESS;
ebpf_result_t result = EBPF_SUCCESS;
net_ebpf_extension_wfp_filter_context_t* local_filter_context = NULL;
FWPM_SESSION session = {0};

NET_EBPF_EXT_LOG_ENTRY();

Expand All @@ -262,13 +264,21 @@ net_ebpf_extension_wfp_filter_context_create(
__fastfail(FAST_FAIL_INVALID_ARG);
}

status = FwpmEngineOpen(NULL, RPC_C_AUTHN_WINNT, NULL, &session, &local_filter_context->wfp_engine_handle);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmEngineOpen", status);

*filter_context = local_filter_context;
local_filter_context = NULL;

Exit:
if (local_filter_context != NULL) {
ExFreePool(local_filter_context);
}

if (status != STATUS_SUCCESS) {
result = EBPF_FAILED;
}

NET_EBPF_EXT_RETURN_RESULT(result);
}

Expand Down Expand Up @@ -352,13 +362,16 @@ net_ebpf_extension_get_callout_id_for_hook(net_ebpf_extension_hook_id_t hook_id)
}
void
net_ebpf_extension_delete_wfp_filters(
uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids)
HANDLE wfp_engine_handle,
uint32_t filter_count,
_Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids)
{
NET_EBPF_EXT_LOG_ENTRY();

NTSTATUS status = STATUS_SUCCESS;

for (uint32_t index = 0; index < filter_count; index++) {
status = FwpmFilterDeleteById(_fwp_engine_handle, filter_ids[index].id);
status = FwpmFilterDeleteById(wfp_engine_handle, filter_ids[index].id);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmFilterDeleteById", status);
Expand Down Expand Up @@ -405,7 +418,7 @@ net_ebpf_extension_add_wfp_filters(

memset(local_filter_ids, 0, (sizeof(net_ebpf_ext_wfp_filter_id_t) * filter_count));

status = FwpmTransactionBegin(_fwp_engine_handle, 0);
status = FwpmTransactionBegin(filter_context->wfp_engine_handle, 0);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionBegin", status);
result = EBPF_INVALID_ARGUMENT;
Expand Down Expand Up @@ -435,7 +448,7 @@ net_ebpf_extension_add_wfp_filters(
REFERENCE_FILTER_CONTEXT(filter_context);
filter.rawContext = (uint64_t)(uintptr_t)filter_context;

status = FwpmFilterAdd(_fwp_engine_handle, &filter, NULL, &local_filter_id);
status = FwpmFilterAdd(filter_context->wfp_engine_handle, &filter, NULL, &local_filter_id);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE_MESSAGE_STRING(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION,
Expand All @@ -458,7 +471,7 @@ net_ebpf_extension_add_wfp_filters(
}
}

status = FwpmTransactionCommit(_fwp_engine_handle);
status = FwpmTransactionCommit(filter_context->wfp_engine_handle);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionCommit", status);
result = EBPF_INVALID_ARGUMENT;
Expand All @@ -474,7 +487,7 @@ net_ebpf_extension_add_wfp_filters(
ExFreePool(local_filter_ids);
}
if (is_in_transaction) {
status = FwpmTransactionAbort(_fwp_engine_handle);
status = FwpmTransactionAbort(filter_context->wfp_engine_handle);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionAbort", status);
Expand Down Expand Up @@ -530,7 +543,7 @@ _net_ebpf_ext_register_wfp_callout(_Inout_ net_ebpf_ext_wfp_callout_state_t* cal
callout_add_state.providerKey = (GUID*)&EBPF_WFP_PROVIDER;
callout_add_state.applicableLayer = *callout_state->layer_guid;

status = FwpmCalloutAdd(_fwp_engine_handle, &callout_add_state, NULL, NULL);
status = FwpmCalloutAdd(_wfp_engine_handle, &callout_add_state, NULL, NULL);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE_MESSAGE_STRING(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION,
Expand Down Expand Up @@ -638,27 +651,32 @@ net_ebpf_extension_initialize_wfp_components(_Inout_ void* device_object)

NET_EBPF_EXT_LOG_ENTRY();

if (_fwp_engine_handle != NULL) {
if (_wfp_engine_handle != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not change the identifier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took it to be an accidental typo ('fwp' instead of 'wfp'), so fixed it. I can certainly revert it back but was curious why you'd prefer the original.

// already registered
goto Exit;
}

// Details @ https://learn.microsoft.com/en-us/windows/win32/fwp/object-management#object-associations.
RtlZeroMemory(&session, sizeof(FWPM_SESSION));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the dynamic flag.

session.flags = FWPM_SESSION_FLAG_DYNAMIC;

status = FwpmEngineOpen(NULL, RPC_C_AUTHN_WINNT, NULL, &session, &_fwp_engine_handle);
status = FwpmEngineOpen(NULL, RPC_C_AUTHN_WINNT, NULL, &session, &_wfp_engine_handle);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmEngineOpen", status);
is_engine_opened = TRUE;

status = FwpmTransactionBegin(_fwp_engine_handle, 0);
status = FwpmTransactionBegin(_wfp_engine_handle, 0);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionBegin", status);
is_in_transaction = TRUE;

// Create the WFP provider.
ebpf_wfp_provider.providerKey = EBPF_WFP_PROVIDER;
ebpf_wfp_provider.displayData.name = L"eBPF for Windows contributors";
ebpf_wfp_provider.displayData.description = L"Windows Networking eBPF Extension";
ebpf_wfp_provider.providerKey = EBPF_WFP_PROVIDER;
status = FwpmProviderAdd(_fwp_engine_handle, &ebpf_wfp_provider, NULL);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmProviderAdd", status);
status = FwpmProviderAdd(_wfp_engine_handle, &ebpf_wfp_provider, NULL);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmProviderAdd", status);
goto Exit;
}

// Add all the sub layers.
for (index = 0; index < EBPF_COUNT_OF(_net_ebpf_ext_sublayers); index++) {
Expand All @@ -671,7 +689,7 @@ net_ebpf_extension_initialize_wfp_components(_Inout_ void* device_object)
ebpf_hook_sub_layer.flags = _net_ebpf_ext_sublayers[index].flags;
ebpf_hook_sub_layer.weight = _net_ebpf_ext_sublayers[index].weight;

status = FwpmSubLayerAdd(_fwp_engine_handle, &ebpf_hook_sub_layer, NULL);
status = FwpmSubLayerAdd(_wfp_engine_handle, &ebpf_hook_sub_layer, NULL);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmSubLayerAdd", status);
}

Expand All @@ -687,7 +705,7 @@ net_ebpf_extension_initialize_wfp_components(_Inout_ void* device_object)
}
}

status = FwpmTransactionCommit(_fwp_engine_handle);
status = FwpmTransactionCommit(_wfp_engine_handle);
NET_EBPF_EXT_BAIL_ON_API_FAILURE_STATUS(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionCommit", status);
is_in_transaction = FALSE;

Expand All @@ -697,10 +715,9 @@ net_ebpf_extension_initialize_wfp_components(_Inout_ void* device_object)
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpsInjectionHandleCreate", status);

Exit:

if (!NT_SUCCESS(status)) {
if (is_in_transaction) {
NTSTATUS abort_status = FwpmTransactionAbort(_fwp_engine_handle);
NTSTATUS abort_status = FwpmTransactionAbort(_wfp_engine_handle);
if (!NT_SUCCESS(abort_status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmTransactionAbort", abort_status);
Expand All @@ -718,33 +735,27 @@ net_ebpf_extension_initialize_wfp_components(_Inout_ void* device_object)
void
net_ebpf_extension_uninitialize_wfp_components(void)
{
size_t index;
NTSTATUS status;

if (_fwp_engine_handle != NULL) {
status = FwpmEngineClose(_fwp_engine_handle);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmEngineClose", status);
}
_fwp_engine_handle = NULL;
NTSTATUS status = STATUS_SUCCESS;

for (index = 0; index < EBPF_COUNT_OF(_net_ebpf_ext_wfp_callout_states); index++) {
status = FwpsCalloutUnregisterById(_net_ebpf_ext_wfp_callout_states[index].assigned_callout_id);
dv-msft marked this conversation as resolved.
Show resolved Hide resolved
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpsCalloutUnregisterById", status);
}
}
}
NET_EBPF_EXT_LOG_ENTRY();

// FwpsInjectionHandleCreate can fail. So, check for NULL.
if (_net_ebpf_ext_l2_injection_handle != NULL) {
status = FwpsInjectionHandleDestroy(_net_ebpf_ext_l2_injection_handle);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpsInjectionHandleDestroy", status);
}
}

if (_wfp_engine_handle != NULL) {
status = FwpmEngineClose(_wfp_engine_handle);
if (!NT_SUCCESS(status)) {
NET_EBPF_EXT_LOG_NTSTATUS_API_FAILURE(NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, "FwpmEngineClose", status);
}
_wfp_engine_handle = NULL;
}

NET_EBPF_EXT_LOG_EXIT();
}

NTSTATUS
Expand Down
33 changes: 22 additions & 11 deletions netebpfext/net_ebpf_ext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ typedef struct _net_ebpf_ext_wfp_filter_id
typedef struct _net_ebpf_extension_wfp_filter_context
{
volatile long reference_count; ///< Reference count.
HANDLE wfp_engine_handle; /// per-filter context WFP handle
const struct _net_ebpf_extension_hook_client* client_context; ///< Pointer to hook NPI client.

net_ebpf_ext_wfp_filter_id_t* filter_ids; ///< Array of WFP filter Ids.
Expand All @@ -106,16 +107,24 @@ typedef struct _net_ebpf_extension_wfp_filter_context
InterlockedIncrement(&(filter_context)->reference_count); \
}

#define DEREFERENCE_FILTER_CONTEXT(filter_context) \
if ((filter_context) != NULL) { \
if (InterlockedDecrement(&(filter_context)->reference_count) == 0) { \
net_ebpf_extension_hook_client_leave_rundown( \
(net_ebpf_extension_hook_client_t*)(filter_context)->client_context); \
if ((filter_context)->filter_ids != NULL) { \
ExFreePool((filter_context)->filter_ids); \
} \
ExFreePool((filter_context)); \
} \
#define DEREFERENCE_FILTER_CONTEXT(filter_context) \
if ((filter_context) != NULL) { \
if (InterlockedDecrement(&(filter_context)->reference_count) == 0) { \
net_ebpf_extension_hook_client_leave_rundown( \
(net_ebpf_extension_hook_client_t*)(filter_context)->client_context); \
if ((filter_context)->filter_ids != NULL) { \
ExFreePool((filter_context)->filter_ids); \
} \
NTSTATUS local_status = FwpmEngineClose((filter_context)->wfp_engine_handle); \
if (!NT_SUCCESS(local_status)) { \
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS( \
NET_EBPF_EXT_TRACELOG_LEVEL_ERROR, \
NET_EBPF_EXT_TRACELOG_KEYWORD_EXTENSION, \
"DEREFERENCE_FILTER_CONTEXT() - FwpmEngineClose() failed", \
local_status); \
} \
ExFreePool((filter_context)); \
} \
}

/**
Expand Down Expand Up @@ -223,7 +232,9 @@ net_ebpf_extension_add_wfp_filters(
*/
void
net_ebpf_extension_delete_wfp_filters(
uint32_t filter_count, _Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids);
HANDLE wfp_engine_handle,
uint32_t filter_count,
dv-msft marked this conversation as resolved.
Show resolved Hide resolved
_Frees_ptr_ _In_count_(filter_count) net_ebpf_ext_wfp_filter_id_t* filter_ids);

// eBPF WFP Provider GUID.
// ddb851f5-841a-4b77-8a46-bb7063e9f162
Expand Down
3 changes: 2 additions & 1 deletion netebpfext/net_ebpf_ext_bind.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ _net_ebpf_extension_bind_on_client_detach(_In_ const net_ebpf_extension_hook_cli
ASSERT(filter_context != NULL);

// Delete the WFP filters.
net_ebpf_extension_delete_wfp_filters(filter_context->filter_ids_count, filter_context->filter_ids);
net_ebpf_extension_delete_wfp_filters(
filter_context->wfp_engine_handle, filter_context->filter_ids_count, filter_context->filter_ids);
net_ebpf_extension_wfp_filter_context_cleanup((net_ebpf_extension_wfp_filter_context_t*)filter_context);
}

Expand Down
3 changes: 2 additions & 1 deletion netebpfext/net_ebpf_ext_sock_addr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,8 @@ _net_ebpf_extension_sock_addr_on_client_detach(_In_ const net_ebpf_extension_hoo
(net_ebpf_extension_sock_addr_wfp_filter_context_t*)net_ebpf_extension_hook_client_get_provider_data(
detaching_client);
ASSERT(filter_context != NULL);
net_ebpf_extension_delete_wfp_filters(filter_context->base.filter_ids_count, filter_context->base.filter_ids);
net_ebpf_extension_delete_wfp_filters(
filter_context->base.wfp_engine_handle, filter_context->base.filter_ids_count, filter_context->base.filter_ids);
if (filter_context->redirect_handle != NULL) {
FwpsRedirectHandleDestroy(filter_context->redirect_handle);
}
Expand Down
3 changes: 2 additions & 1 deletion netebpfext/net_ebpf_ext_sock_ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ _net_ebpf_extension_sock_ops_on_client_detach(_In_ const net_ebpf_extension_hook

ASSERT(filter_context != NULL);
InitializeListHead(&local_list_head);
net_ebpf_extension_delete_wfp_filters(filter_context->base.filter_ids_count, filter_context->base.filter_ids);
net_ebpf_extension_delete_wfp_filters(
filter_context->base.wfp_engine_handle, filter_context->base.filter_ids_count, filter_context->base.filter_ids);

KeAcquireSpinLock(&filter_context->lock, &irql);
if (filter_context->flow_context_list.count > 0) {
Expand Down
3 changes: 2 additions & 1 deletion netebpfext/net_ebpf_ext_xdp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ _net_ebpf_extension_xdp_on_client_detach(_In_ const net_ebpf_extension_hook_clie
NET_EBPF_EXT_LOG_ENTRY();

ASSERT(filter_context != NULL);
net_ebpf_extension_delete_wfp_filters(filter_context->base.filter_ids_count, filter_context->base.filter_ids);
net_ebpf_extension_delete_wfp_filters(
filter_context->base.wfp_engine_handle, filter_context->base.filter_ids_count, filter_context->base.filter_ids);
net_ebpf_extension_wfp_filter_context_cleanup((net_ebpf_extension_wfp_filter_context_t*)filter_context);

NET_EBPF_EXT_LOG_EXIT();
Expand Down
Loading