Skip to content

Commit

Permalink
Bail if we can't enter rundown (not a fatal error in some instances) (#…
Browse files Browse the repository at this point in the history 
…3148)

Co-authored-by: Dhiren Vispute <Dhiren.Vispute@microsoft.com>
  • Loading branch information
@dv-msft
dv-msft and Dhiren Vispute committed Jan 9, 2024
1 parent c538729 commit a2ece4b
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 40 deletions.
9 changes: 8 additions & 1 deletion netebpfext/net_ebpf_ext.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,14 @@ net_ebpf_extension_wfp_filter_context_create(
memset(local_filter_context, 0, filter_context_size);
local_filter_context->reference_count = 1; // Initial reference.
local_filter_context->client_context = client_context;
ENTER_HOOK_CLIENT_RUNDOWN((net_ebpf_extension_hook_client_t*)local_filter_context->client_context);

if (!net_ebpf_extension_hook_client_enter_rundown(
(net_ebpf_extension_hook_client_t*)local_filter_context->client_context)) {

// We're setting up the filter context here and as this is the very first (and exclusive) attempt to acquire
// rundown, it cannot fail. If it does, this is indicative of a fatal system level error.
__fastfail(FAST_FAIL_INVALID_ARG);
}

*filter_context = local_filter_context;
local_filter_context = NULL;
Expand Down
25 changes: 0 additions & 25 deletions netebpfext/net_ebpf_ext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,31 +98,6 @@ typedef struct _net_ebpf_extension_wfp_filter_context
} \
}

#define ENTER_HOOK_CLIENT_RUNDOWN(hook_client) \
{ \
/* \
* In certain locations, invocation of the following call cannot ever return false. We always have a valid \
* filter_context in these locations so this call cannot fail as the hook_client rundown object is also used \
* to ensure validity of the filter_context. Using an additional rundown object for this purpose would be \
* overkill and and inefficient use of already available resources. \
* \
* In such instances, a 'false' return value indicates a fatal internal error. This convenience macro checks \
* for this condition and triggers an immediate bugcheck. \
* \
* To be clear, it is ok to call the underlying call directly in other use cases that _do_ _not_ involve \
* accessing the filter_context, with the explicit caveat that the caller be thoroughly aware of the \
* consequences of this call returning false. \
*/ \
if (!net_ebpf_extension_hook_client_enter_rundown((hook_client))) { \
__fastfail(FAST_FAIL_INVALID_ARG); \
} \
}

#define LEAVE_HOOK_CLIENT_RUNDOWN(hook_client) \
{ \
net_ebpf_extension_hook_client_leave_rundown((hook_client)); \
}

/**
* @brief This function allocates and initializes a net ebpf extension WFP filter context. This should be invoked when
* the hook client is being attached.
Expand Down
24 changes: 20 additions & 4 deletions netebpfext/net_ebpf_ext_bind.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,15 @@ net_ebpf_ext_resource_allocation_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_BIND,
"net_ebpf_ext_resource_allocation_classify - Client detach detected.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

addr.sin_port =
incoming_fixed_values->incomingValue[FWPS_FIELD_ALE_RESOURCE_ASSIGNMENT_V4_IP_LOCAL_PORT].value.uint16;
Expand Down Expand Up @@ -313,7 +321,7 @@ net_ebpf_ext_resource_allocation_classify(

Exit:
if (attached_client) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}
return;
}
Expand Down Expand Up @@ -356,7 +364,15 @@ net_ebpf_ext_resource_release_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_BIND,
"net_ebpf_ext_resource_release_classify - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

addr.sin_port = incoming_fixed_values->incomingValue[FWPS_FIELD_ALE_RESOURCE_RELEASE_V4_IP_LOCAL_PORT].value.uint16;
addr.sin_addr.S_un.S_addr =
Expand All @@ -383,7 +399,7 @@ net_ebpf_ext_resource_release_classify(

Exit:
if (attached_client) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}
return;
}
Expand Down
24 changes: 20 additions & 4 deletions netebpfext/net_ebpf_ext_sock_addr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,15 @@ net_ebpf_extension_sock_addr_authorize_recv_accept_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->base.client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_SOCK_ADDR,
"net_ebpf_extension_sock_addr_authorize_recv_accept_classify - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

_net_ebpf_extension_sock_addr_copy_wfp_connection_fields(
incoming_fixed_values, incoming_metadata_values, &net_ebpf_sock_addr_ctx);
Expand Down Expand Up @@ -1217,7 +1225,7 @@ net_ebpf_extension_sock_addr_authorize_recv_accept_classify(

Exit:
if (attached_client) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}

NET_EBPF_EXT_LOG_EXIT();
Expand Down Expand Up @@ -1550,7 +1558,15 @@ net_ebpf_extension_sock_addr_redirect_connection_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->base.client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_SOCK_ADDR,
"net_ebpf_extension_sock_addr_redirect_connection_classify - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

// Get the redirect handle for this filter.
redirect_handle = filter_context->redirect_handle;
Expand Down Expand Up @@ -1693,7 +1709,7 @@ net_ebpf_extension_sock_addr_redirect_connection_classify(
}

if (attached_client) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}

if (net_ebpf_sock_addr_ctx.redirect_context != NULL) {
Expand Down
24 changes: 20 additions & 4 deletions netebpfext/net_ebpf_ext_sock_ops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,15 @@ net_ebpf_extension_sock_ops_flow_established_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->base.client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_SOCK_OPS,
"net_ebpf_extension_sock_ops_flow_established_classify - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

local_flow_context = (net_ebpf_extension_sock_ops_wfp_flow_context_t*)ExAllocatePoolUninitialized(
NonPagedPoolNx, sizeof(net_ebpf_extension_sock_ops_wfp_flow_context_t), NET_EBPF_EXTENSION_POOL_TAG);
Expand Down Expand Up @@ -500,7 +508,7 @@ net_ebpf_extension_sock_ops_flow_established_classify(
ExFreePool(local_flow_context);
}
if (attached_client != NULL) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}
}

Expand Down Expand Up @@ -533,7 +541,15 @@ net_ebpf_extension_sock_ops_flow_delete(uint16_t layer_id, uint32_t callout_id,
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->base.client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_SOCK_OPS,
"net_ebpf_extension_sock_ops_flow_delete - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

KeAcquireSpinLock(&filter_context->lock, &irql);
RemoveEntryList(&local_flow_context->link);
Expand Down Expand Up @@ -563,7 +579,7 @@ net_ebpf_extension_sock_ops_flow_delete(uint16_t layer_id, uint32_t callout_id,
}

if (attached_client != NULL) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}
}

Expand Down
12 changes: 10 additions & 2 deletions netebpfext/net_ebpf_ext_xdp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,15 @@ net_ebpf_ext_layer_2_classify(
}

attached_client = (net_ebpf_extension_hook_client_t*)filter_context->base.client_context;
ENTER_HOOK_CLIENT_RUNDOWN(attached_client);
if (!net_ebpf_extension_hook_client_enter_rundown(attached_client)) {
NET_EBPF_EXT_LOG_MESSAGE_NTSTATUS(
NET_EBPF_EXT_TRACELOG_LEVEL_VERBOSE,
NET_EBPF_EXT_TRACELOG_KEYWORD_XDP,
"net_ebpf_ext_layer_2_classify - Rundown already started.",
STATUS_INVALID_PARAMETER);
attached_client = NULL;
goto Exit;
}

if (nbl == NULL) {
NET_EBPF_EXT_LOG_MESSAGE(NET_EBPF_EXT_TRACELOG_LEVEL_ERROR, NET_EBPF_EXT_TRACELOG_KEYWORD_XDP, "Null NBL");
Expand Down Expand Up @@ -772,7 +780,7 @@ net_ebpf_ext_layer_2_classify(

Exit:
if (attached_client) {
LEAVE_HOOK_CLIENT_RUNDOWN(attached_client);
net_ebpf_extension_hook_client_leave_rundown(attached_client);
}
}

Expand Down

0 comments on commit a2ece4b

Please sign in to comment.