Merge pull request #135 from neuvector/test_bpf_versions

fix: solve some verifier issues and introduce a bpf CI

Author: Andreagit97

.github/actions/bpfvalidator/action.yml

@@ -0,0 +1,77 @@
+name: 'bpfvalidator'
+description: 'Run eBPF programs against a range of kernels.'
+
+inputs:
+  arch:
+    description: 'Architecture to test (e.g., amd64, arm64)'
+    required: true
+    type: string
+  args:
+    description: 'Command line to pass to bpfvalidator'
+    required: true
+    type: string
+  fail_on_validation:
+    description: 'fail the action if validation fails'
+    required: false
+    default: true
+    type: boolean
+  skip_dependencies:
+    description: 'skip installing dependencies'
+    required: false
+    default: false
+    type: boolean
+
+outputs:
+  report:
+    description: "Report of the validation"
+    value: ${{ steps.run-validator.outputs.report }}
+  outcome:
+    description: "Outcome of the validation"
+    value: ${{ steps.run-validator.outputs.outcome }}
+
+runs:
+  using: "composite"
+  steps:
+    # we hardcode the vng version here, since dependencies and build process could change among different versions, we shouldn't let the user choose the version
+    - name: Install deps
+      if: inputs.skip_dependencies != 'true'
+      shell: bash
+      run: |
+        sudo apt update
+        sudo apt install -y git qemu-system udev virtiofsd
+        git clone --single-branch --branch v1.36 --depth 1 --recurse-submodules https://github.com/arighi/virtme-ng.git
+        cd virtme-ng/
+        BUILD_VIRTME_NG_INIT=1 pip3 install . --break-system-packages
+        echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+    # Try to enable KVM if available
+    - name: KVM support
+      shell: bash
+      continue-on-error: true
+      run: |
+        echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules
+        sudo udevadm control --reload-rules
+        sudo udevadm trigger --name-match=kvm
+        sudo apt install -y qemu-kvm kmod
+
+    # we offer 2 possible ways to consume the output of bpfvalidator:
+    # 1. print it directly in the action log
+    # 2. set it as an output variable      
+    - name: Run bpfvalidator
+      # we don't want the action to fail immediately if bpfvalidator fails.
+      # so we remove `-eo pipefail` from the shell option.
+      # see https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference
+      shell: bash --noprofile --norc {0}
+      id: run-validator
+      working-directory: ${{ github.action_path }}
+      run: |
+        wget https://github.com/Andreagit97/bpfvalidator/releases/download/v0.3.0/bpfvalidator_0.3.0_linux_${{ inputs.arch }}.tar.gz
+        tar -xvf bpfvalidator_0.3.0_linux_${{ inputs.arch }}.tar.gz
+        ./bpfvalidator --out_path=/tmp/report.txt ${{ inputs.args }}
+        outcome=$?
+        echo "outcome=$outcome" >> $GITHUB_OUTPUT
+        echo "report=/tmp/report.txt" >> $GITHUB_OUTPUT
+        cat /tmp/report.txt
+        if [ $outcome -ne 0 ] && [ "${{ inputs.fail_on_validation }}" == "true" ]; then
+          exit $outcome
+        fi

.github/workflows/bpf_test.yml

@@ -0,0 +1,39 @@
+name: bpf tests
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  test:
+    name: test-${{ matrix.arch }}
+    runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-24.04-arm') || 'ubuntu-24.04' }}
+    strategy:
+      fail-fast: false
+      matrix:
+        arch: [amd64]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+      - uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0
+        with:
+          go-version-file: "go.mod"
+      - name: Install system dependencies for eBPF build
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y \
+            build-essential \
+            libelf-dev \
+            clang \
+            llvm \
+            libbpf-dev
+      - name: Generate eBPF code
+        run: |
+          make generate-ebpf
+          go test -c ./internal/bpf/... -o tester
+      - name: Validate bpf tests using bpfvalidator
+        uses: ./.github/actions/bpfvalidator
+        with:
+          args: |
+            --config=$GITHUB_WORKSPACE/bpfvalidator-${{ matrix.arch }}-config.yaml --cmd="$GITHUB_WORKSPACE/tester -test.v"
+          arch: ${{ matrix.arch }}

bpf/d_path_resolution.h

@@ -4,17 +4,20 @@
 // kernel's max dentry name length that is 255
 // (https://elixir.bootlin.com/linux/v5.10/source/include/uapi/linux/limits.h#L12) + 1 for the `/`
 #define MAX_COMPONENT_LEN 256
-// Max iterations when unrolling loops
-#define UNROLL_PATH_ITERATIONS 128
-// Max iterations when looping paths
-#define LOOP_PATH_ITERATIONS 2048
+// Max iterations when looping paths, we can reach at least 1024 but the verification time
+// increases, so for now we keep it conservative, and moreover 512 should be more than enough.
+#define FALLBACK_PATH_ITERATIONS 512
+// With numeric code iterators we have no limits.
+#define PATH_ITERATIONS 2048
 
 #define DELETED_STRING " (deleted)"
 
 #define SAFE_PATH_LEN(x) (x) & (MAX_PATH_LEN - 1)
 #define SAFE_PATH_ACCESS(x) (x) & (MAX_PATH_LEN * 2 - 1)
 #define SAFE_COMPONENT_ACCESS(x) (x) & (MAX_COMPONENT_LEN - 1)
 
+extern int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end) __ksym __weak;
+
 struct path_read_data {
 	struct dentry *root_dentry;
 	struct vfsmount *root_mnt;
@@ -93,10 +96,6 @@ static __always_inline long path_read(struct path_read_data *data) {
 	return 0;
 }
 
-static long path_read_loop(__u32 index, void *data) {
-	return path_read(data);
-}
-
 // this method is inspired by Tetragon https://github.com/cilium/tetragon/pull/90
 // but simplified and reworked in light of our specific use case
 static __always_inline int bpf_d_path_approx(const struct path *path, char *buf) {
@@ -134,11 +133,25 @@ static __always_inline int bpf_d_path_approx(const struct path *path, char *buf)
 	        struct mount,
 	        mnt);  // container_of comes from bpf_helpers.h and it is already adapted for CO-RE
 
-	if(bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_loop)) {
-		bpf_loop(LOOP_PATH_ITERATIONS, path_read_loop, (void *)&data, 0);
+	if(bpf_ksym_exists(bpf_iter_num_new)) {
+		// Numeric code iterators are available from kernel 6.4
+		// (https://docs.ebpf.io/linux/kfuncs/bpf_iter_num_new/) so we check if the kfunc is
+		// available.
+		// `bpf_repeat` is a macro defined by libbpf that uses `bpf_iter_num_new` under
+		// the hood.
+		// The initial implementation used `bpf_loop`, but this is not so handy to use with CO-RE,
+		// you can find more info here
+		// https://lore.kernel.org/bpf/CAGQdkDt9zyQwr5JyftXqL=OLKscNcqUtEteY4hvOkx2S4GdEkQ@mail.gmail.com/T/#u
+		// and here https://github.com/falcosecurity/libs/pull/2027#issuecomment-2568997393
+		// TL;DR; we need 2 ebpf programs, one with `bpf_loop` on kernels >= 5.13 and another
+		// without it on older kernels.
+		bpf_repeat(PATH_ITERATIONS) {
+			if(path_read(&data)) {
+				break;
+			}
+		}
 	} else {
-#pragma unroll
-		for(int i = 0; i < UNROLL_PATH_ITERATIONS; ++i) {
+		for(int i = 0; i < FALLBACK_PATH_ITERATIONS; ++i) {
 			if(path_read(&data)) {
 				break;
 			}

bpf/main.c

@@ -24,6 +24,7 @@ extern int LINUX_KERNEL_VERSION __kconfig;
 struct {
 	__uint(type, BPF_MAP_TYPE_HASH);
 	__uint(max_entries, TRACKER_MAP_MAX_ENTRIES);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
 	__type(key, __u64);   /* cgroup id */
 	__type(value, __u64); /* tracker cgroup id */
 } cgtracker_map SEC(".maps");
@@ -426,11 +427,6 @@ static __always_inline u16 string_padded_len(u16 len) {
 
 	if(len <= STRING_MAPS_SIZE_6)
 		return STRING_MAPS_SIZE_6;
-
-	if(LINUX_KERNEL_VERSION < KERNEL_VERSION(5, 11, 0)) {
-		return STRING_MAPS_SIZE_7;
-	}
-
 	if(len <= STRING_MAPS_SIZE_7)
 		return STRING_MAPS_SIZE_7;
 	if(len <= STRING_MAPS_SIZE_8)
@@ -441,13 +437,8 @@ static __always_inline u16 string_padded_len(u16 len) {
 }
 
 static __always_inline int string_map_index(u16 padded_len) {
-	if(padded_len < STRING_MAPS_SIZE_5)
+	if(padded_len < STRING_MAPS_SIZE_5) {
 		return (padded_len / STRING_MAPS_KEY_INC_SIZE) - 1;
-
-	if(LINUX_KERNEL_VERSION < KERNEL_VERSION(5, 11, 0)) {
-		if(padded_len == STRING_MAPS_SIZE_6)
-			return 6;
-		return 7;
 	}
 
 	switch(padded_len) {
@@ -515,6 +506,7 @@ int BPF_PROG(enforce_cgroup_policy, struct linux_binprm *bprm) {
 
 	// Only 5.11+ kernels support hash key lengths > 512 bytes
 	// https://github.com/cilium/tetragon/commit/834b5fe7d4063928cf7b89f61252637d833ca018
+	// From now on, we are sure that evt->path_len is <= STRING_MAPS_SIZE_7 if on <5.11
 	if(LINUX_KERNEL_VERSION < KERNEL_VERSION(5, 11, 0)) {
 		if(evt->path_len > STRING_MAPS_SIZE_7) {
 			bpf_printk("Path length %d exceeds max supported length", evt->path_len);

bpf/string_maps.h

@@ -1,6 +1,7 @@
 #pragma once
 
-#define POLICY_STR_OUTER_MAX_ENTRIES 1
+// we will decrese the number of entries in userspace if the map is not used (<5.11)
+#define POLICY_STR_OUTER_MAX_ENTRIES 65536
 #define POLICY_STR_INNER_MAX_ENTRIES 1
 
 /* Taken and adapted from https://github.com/cilium/tetragon/pull/1408
@@ -48,18 +49,6 @@
 #define STRING_MAPS_SIZE_9 (2048)
 #define STRING_MAPS_SIZE_10 (4096)
 
-// todo!: we want to compile only once so we should avoid the ifdef. We should probably not create
-// maps > 8. Test it, if it is feasible
-//
-// #ifdef __V511_BPF_PROG
-// #define STRING_MAPS_SIZE_7  (512 + 2)
-// #define STRING_MAPS_SIZE_8  (1024 + 2)
-// #define STRING_MAPS_SIZE_9  (2048 + 2)
-// #define STRING_MAPS_SIZE_10 (4096 + 2)
-// #else
-// #define STRING_MAPS_SIZE_7 (512)
-// #endif
-
 #define DEFINE_POLICY_STR_HASH_OF_MAPS(N)                              \
 	struct {                                                           \
 		__uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);                       \
@@ -115,10 +104,3 @@ static __always_inline void* get_policy_string_map(int index, u64* policy_id) {
 	}
 	return 0;
 }
-
-// todo!: same as before for `__V511_BPF_PROG`
-// #ifdef __V511_BPF_PROG
-// DEFINE_POLICY_STR_HASH_OF_MAPS(8)
-// DEFINE_POLICY_STR_HASH_OF_MAPS(9)
-// DEFINE_POLICY_STR_HASH_OF_MAPS(10)
-// #endif

bpfvalidator-amd64-config.yaml

@@ -0,0 +1,11 @@
+# Number of parallel VMs to run
+parallel: 4
+# kernel versions to test
+kernel_versions:
+    - v5.10.247
+    - v5.15.197
+    - v6.1.159
+    - v6.6.119
+    - v6.12.62
+    - v6.17.12
+    - v6.18.1

internal/bpf/manager.go

@@ -6,8 +6,13 @@ import (
 	"log/slog"
 
 	"github.com/cilium/ebpf"
+	"github.com/cilium/ebpf/asm"
+	"github.com/cilium/ebpf/features"
+	"github.com/cilium/ebpf/link"
 	"github.com/cilium/ebpf/rlimit"
 	"github.com/neuvector/runtime-enforcer/internal/cgroups"
+	"github.com/neuvector/runtime-enforcer/internal/kernels"
+
 	"golang.org/x/sync/errgroup"
 )
 
@@ -17,6 +22,9 @@ import (
 
 const (
 	loadTimeConfigBPFVar = "load_time_config"
+	policyMap8Name       = "pol_str_maps_8"
+	policyMap9Name       = "pol_str_maps_9"
+	policyMap10Name      = "pol_str_maps_10"
 )
 
 const (
@@ -53,11 +61,58 @@ type Manager struct {
 	monitoringEventChan chan ProcessEvent
 }
 
+func probeEbpfFeatures() error {
+	// For now known requirements are:
+	// - BPF_MAP_TYPE_RINGBUF
+	// - tracing prog with attach type BPF_MODIFY_RETURN
+
+	// Check for BPF_MAP_TYPE_RINGBUF
+	if err := features.HaveMapType(ebpf.RingBuf); err != nil {
+		return fmt.Errorf("BPF_MAP_TYPE_RINGBUF not supported: %w", err)
+	}
+
+	// Check for BPF_MODIFY_RETURN attach type
+	// Today there is no an helper function for attach type BPF_MODIFY_RETURN so we do it by hand.
+	prog, err := ebpf.NewProgram(&ebpf.ProgramSpec{
+		Name: "probe_fmodret",
+		Type: ebpf.Tracing,
+		Instructions: asm.Instructions{
+			asm.Mov.Imm(asm.R0, 0),
+			asm.Return(),
+		},
+		AttachType: ebpf.AttachModifyReturn,
+		License:    "MIT",
+		AttachTo:   "security_bprm_creds_for_exec",
+	})
+	if err != nil {
+		return err
+	}
+	defer prog.Close()
+
+	link, err := link.AttachTracing(link.TracingOptions{
+		Program: prog,
+	})
+	if err != nil {
+		return err
+	}
+	err = link.Close()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func NewManager(logger *slog.Logger, enableLearning bool, eBPFLogLevel ebpf.LogLevel) (*Manager, error) {
 	if err := rlimit.RemoveMemlock(); err != nil {
 		return nil, fmt.Errorf("failed to remove memlock: %w", err)
 	}
 
+	logger.Info("Probing eBPF features...")
+	if err := probeEbpfFeatures(); err != nil {
+		return nil, fmt.Errorf("failure during eBPF feature probing: %w", err)
+	}
+
 	spec, err := loadBpf()
 	if err != nil {
 		return nil, fmt.Errorf("failed to load BPF spec: %w", err)
@@ -78,6 +133,25 @@ func NewManager(logger *slog.Logger, enableLearning bool, eBPFLogLevel ebpf.LogL
 		"cgrp_v1_subsys_idx", conf.Cgrpv1SubsysIdx,
 		"debug_mode", conf.DebugMode)
 
+	// Only kernels >= 5.11 support hash key lengths > 512 bytes
+	// https://github.com/cilium/tetragon/commit/834b5fe7d4063928cf7b89f61252637d833ca018
+	// so we reduce the key size for older kernels, these maps won't be used anyway
+	if kernels.CurrVersionIsLowerThan("5.11") {
+		for _, mapName := range []string{policyMap8Name, policyMap9Name, policyMap10Name} {
+			policyMap, ok := spec.Maps[mapName]
+			if !ok {
+				return nil, fmt.Errorf("map %s not found in spec", mapName)
+			}
+			// Entries should be already set to 1 in the spec, but just in case
+			policyMap.MaxEntries = 1
+			if policyMap.InnerMap == nil {
+				return nil, fmt.Errorf("map %s is not a hash of maps", mapName)
+			}
+			// this is the max key size supported on older kernels
+			policyMap.InnerMap.KeySize = stringMapSize7
+		}
+	}
+
 	// We just load the objects here so that we can pass the maps to other components but we don't load ebpf progs yet
 	objs := bpfObjects{}
 	opts := &ebpf.CollectionOptions{