Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions doc/usage/bfcli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ With:
- ``BF_HOOK_NF_LOCAL_OUT``: similar to ``nftables`` and ``iptables`` output hook.
- ``BF_HOOK_NF_POST_ROUTING``: similar to ``nftables`` and ``iptables`` postrouting hook.
- ``BF_HOOK_TC_EGRESS``: egress TC hook.
- ``BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4``: cgroup hook for IPv4 ``connect()`` syscalls.
- ``BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6``: cgroup hook for IPv6 ``connect()`` syscalls.
- ``$POLICY``: action taken if no rule matches the packet, either ``ACCEPT`` forward the packet to the kernel, or ``DROP`` to discard it. Note while ``CONTINUE`` is a valid verdict for rules, it is not supported for chain policy.

``$OPTIONS`` are hook-specific comma separated key value pairs. A given hook option can only be specified once:
Expand All @@ -358,7 +360,7 @@ With:
- N/A
- Interface index to attach the program to.
* - ``cgpath=$CGROUP_PATH``
- ``BF_HOOK_CGROUP_SKB_INGRESS``, ``BF_HOOK_CGROUP_SKB_EGRESS``
- ``BF_HOOK_CGROUP_SKB_INGRESS``, ``BF_HOOK_CGROUP_SKB_EGRESS``, ``BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4``, ``BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6``
- N/A
- Path to the cgroup to attach to.
* - ``family=$FAMILY``
Expand Down Expand Up @@ -389,7 +391,7 @@ Rules are defined such as:
With:
- ``$MATCHER``: zero or more matchers. Matchers are defined later.
- ``log``: optional. If set, log the requested protocol headers. ``link`` will log the link (layer 2) header, ``internet`` with log the internet (layer 3) header, and ``transport`` will log the transport (layer 4) header. At least one header type is required.
- ``counter``: optional literal. If set, the filter will counter the number of packets and bytes matched by the rule.
- ``counter``: optional literal. If set, the filter will count the number of events matched by the rule. For packet-based hooks, this includes both the number of packets and the total bytes. For ``BF_HOOK_CGROUP_SOCK_ADDR_*`` hooks, this counts the number of ``connect()`` calls.
- ``mark``: optional, ``$MARK`` must be a valid decimal or hexadecimal 32-bits value. If set, write the packet's marker value. This marker can be used later on in a rule (see ``meta.mark``) or with a TC filter.
- ``$VERDICT``: action taken by the rule if the packet is matched against **all** the criteria: either ``ACCEPT``, ``DROP``, ``CONTINUE``, or ``REDIRECT``.
- ``ACCEPT``: forward the packet to the kernel.
Expand All @@ -407,7 +409,7 @@ Note ``CONTINUE`` means a packet can be counted more than once if multiple rules
- ``BF_HOOK_XDP``: only ``out`` direction is supported (XDP always transmits out of the target interface).
- ``BF_HOOK_TC_INGRESS``, ``BF_HOOK_TC_EGRESS``: both ``in`` and ``out`` directions are supported.

``REDIRECT`` is **not** supported by Netfilter (``BF_HOOK_NF_*``) or cgroup_skb (``BF_HOOK_CGROUP_SKB_*``) hooks.
``REDIRECT`` is **not** supported by Netfilter (``BF_HOOK_NF_*``), cgroup_skb (``BF_HOOK_CGROUP_SKB_*``), or cgroup_sock_addr (``BF_HOOK_CGROUP_SOCK_ADDR_*``) hooks.

Sets
~~~~
Expand Down Expand Up @@ -539,7 +541,7 @@ Meta
- ``meta.probability``
- ``eq``
- ``$PROBABILITY``
- ``$PROBABILITY`` is a floating-point percentage value (i.e., within [0%, 100%], e.g., "50%" or "33.33%").
- ``$PROBABILITY`` is a floating-point percentage value (i.e., within [0%, 100%], e.g., "50%" or "33.33%"). For ``BF_HOOK_CGROUP_SOCK_ADDR_*`` hooks, probability applies at the connection level: each ``connect()`` call is independently accepted or dropped.
* - :rspan:`1` Mark
- :rspan:`1` ``meta.mark``
- ``eq``
Expand Down
2 changes: 1 addition & 1 deletion doc/usage/daemon.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ Namespaces

The network namespace will define the available interface indexes to attach the XDP and TC chains, as well as the interface indexes to filter packets on.

The mount namespace is required to ensure the daemon will attach a cgroup_skb chain to the proper cgroup.
The mount namespace is required to ensure the daemon will attach a cgroup_skb or cgroup_sock_addr chain to the proper cgroup.
2 changes: 1 addition & 1 deletion src/bfcli/lexer.l
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ set { return SET; }
counter { return COUNTER; }

/* Hooks */
BF_HOOK_[A-Z_]+ { BEGIN(STATE_HOOK_OPTS); yylval.sval = strdup(yytext); return HOOK; }
BF_HOOK_[A-Z0-9_]+ { BEGIN(STATE_HOOK_OPTS); yylval.sval = strdup(yytext); return HOOK; }
<STATE_HOOK_OPTS>{
(\{|,) /* Ignore */
\} { BEGIN(INITIAL); }
Expand Down
29 changes: 23 additions & 6 deletions src/bfcli/print.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ void bfc_chain_dump(struct bf_chain *chain, struct bf_hookopts *hookopts,
struct bf_counter *counter;
bf_list_node *counter_node, *policy_counter_node, *err_counter_node;
bool need_comma = false;
bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;
Comment on lines +89 to +90
Copy link

Choose a reason for hiding this comment

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

Claude: Pointer dereference before assert() precondition check

is_pkt_hook dereferences chain->hook at line 89 before assert(chain) validates the pointer at line 92. The project convention (confirmed across _bf_matcher_generate_ip4_addr, bf_matcher_generate_packet, etc.) places assert() checks before any use of the validated pointer.

Suggested change
bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;
bool is_pkt_hook;

Then assign after the asserts:

    assert(chain);
    assert(counters);

    is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
                  chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;


assert(chain);
assert(counters);
Expand Down Expand Up @@ -137,12 +139,21 @@ void bfc_chain_dump(struct bf_chain *chain, struct bf_hookopts *hookopts,
(void)fprintf(stdout, " %s\n", bf_verdict_to_str(chain->policy));

counter = bf_list_node_get_data(policy_counter_node);
(void)fprintf(stdout, " counters policy %lu packets %lu bytes; ",
counter->count, counter->size);
if (is_pkt_hook) {
(void)fprintf(stdout, " counters policy %lu packets %lu bytes; ",
counter->count, counter->size);
} else {
(void)fprintf(stdout, " counters policy %lu calls; ",
counter->count);
}

counter = bf_list_node_get_data(err_counter_node);
(void)fprintf(stdout, "error %lu packets %lu bytes\n", counter->count,
counter->size);
if (is_pkt_hook) {
(void)fprintf(stdout, "error %lu packets %lu bytes\n", counter->count,
counter->size);
} else {
(void)fprintf(stdout, "error %lu calls\n", counter->count);
}

// Loop over named sets
bf_list_foreach (&chain->sets, set_node) {
Expand Down Expand Up @@ -270,8 +281,14 @@ void bfc_chain_dump(struct bf_chain *chain, struct bf_hookopts *hookopts,

if (rule->counters) {
counter = bf_list_node_get_data(counter_node);
(void)fprintf(stdout, " counters %lu packets %lu bytes\n",
counter->count, counter->size);
if (is_pkt_hook) {
(void)fprintf(stdout,
" counters %lu packets %lu bytes\n",
counter->count, counter->size);
} else {
(void)fprintf(stdout, " counters %lu calls\n",
counter->count);
}
}
counter_node = bf_list_node_next(counter_node);

Expand Down
8 changes: 3 additions & 5 deletions src/bpfilter/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,16 @@ add_executable(bpfilter
${CMAKE_CURRENT_SOURCE_DIR}/main.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgen.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgen.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgroup_skb.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgroup_skb.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgroup_sock_addr.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/cgroup_sock_addr.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/dump.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/dump.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/elfstub.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/elfstub.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/fixup.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/fixup.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/handle.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/handle.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/jmp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/jmp.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip4.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip4.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip6.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/ip6.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/tcp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/tcp.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/udp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/udp.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/cmp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/cmp.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/meta.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/meta.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/packet.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/packet.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/set.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/set.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/icmp.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/matcher/icmp.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/nf.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/nf.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/printer.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/printer.c
${CMAKE_CURRENT_SOURCE_DIR}/cgen/program.h ${CMAKE_CURRENT_SOURCE_DIR}/cgen/program.c
Expand Down
47 changes: 28 additions & 19 deletions src/bpfilter/cgen/cgroup_skb.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@
#include <bpfilter/btf.h>
#include <bpfilter/flavor.h>
#include <bpfilter/helper.h>
#include <bpfilter/matcher.h>
#include <bpfilter/verdict.h>

#include "cgen/cgen.h"
#include "cgen/matcher/cmp.h"
#include "cgen/matcher/meta.h"
#include "cgen/matcher/packet.h"
#include "cgen/program.h"
#include "cgen/stub.h"
#include "cgen/swich.h"
Expand Down Expand Up @@ -114,30 +118,36 @@ static int _bf_cgroup_skb_gen_inline_set_mark(struct bf_program *program,
return 0;
}

static int _bf_cgroup_skb_gen_inline_get_mark(struct bf_program *program,
int reg)
static int _bf_cgroup_skb_gen_inline_matcher(struct bf_program *program,
const struct bf_matcher *matcher)
{
EMIT(program,
BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_10, BF_PROG_CTX_OFF(arg)));
EMIT(program,
BPF_LDX_MEM(BPF_W, reg, BPF_REG_1, offsetof(struct __sk_buff, mark)));

return 0;
}

static int _bf_cgroup_skb_gen_inline_get_skb(struct bf_program *program,
int reg)
{
EMIT(program, BPF_LDX_MEM(BPF_DW, reg, BPF_REG_10, BF_PROG_CTX_OFF(arg)));

return 0;
assert(program);
assert(matcher);

switch (bf_matcher_get_type(matcher)) {
case BF_MATCHER_META_MARK:
EMIT(program,
BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_10, BF_PROG_CTX_OFF(arg)));
EMIT(program, BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1,
offsetof(struct __sk_buff, mark)));

return bf_cmp_value(program, bf_matcher_get_op(matcher),
bf_matcher_payload(matcher), 4);
case BF_MATCHER_META_FLOW_HASH:
EMIT(program,
BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_10, BF_PROG_CTX_OFF(arg)));

return bf_matcher_generate_meta_flow_hash(program, matcher);
default:
return bf_matcher_generate_packet(program, matcher);
}
}

/**
* Convert a standard verdict into a return value.
*
* @param verdict Verdict to convert. Must be valid.
* @return TC return code corresponding to the verdict, as an integer.
* @return Cgroup return code corresponding to the verdict, as an integer.
*/
static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict)
{
Expand All @@ -155,7 +165,6 @@ const struct bf_flavor_ops bf_flavor_ops_cgroup_skb = {
.gen_inline_prologue = _bf_cgroup_skb_gen_inline_prologue,
.gen_inline_epilogue = _bf_cgroup_skb_gen_inline_epilogue,
.gen_inline_set_mark = _bf_cgroup_skb_gen_inline_set_mark,
.gen_inline_get_mark = _bf_cgroup_skb_gen_inline_get_mark,
.gen_inline_get_skb = _bf_cgroup_skb_gen_inline_get_skb,
.get_verdict = _bf_cgroup_skb_get_verdict,
.gen_inline_matcher = _bf_cgroup_skb_gen_inline_matcher,
};
Loading
Loading