Build ebpfcore as a DLL for testing #2744

dthaler · 2023-08-09T22:59:18Z

Description

Update to latest usersim
Fix bug in libbpf_link.cpp found by latest usersim, where ebpf_free() was being used to free memory allocated by strdup() rather than an ebpf allocator
Fix memory leaks in libbpf_program.cpp where strdup() was used to allocate memory that was never freed
Fix bugs in test_helper.cpp found by latest usersim, where ebpf_free() was being used to free memory allocated by new.
Fixed bug found by latest usersim, where ebpf_program.c used ebpf_free() to free memory allocated by ubpf_user.c using calloc(). ubpf_kernel.c already redefined it to use ebpf_alloc but ubpf_user.c didn't match.
Fix mismatched alloc/free in verifier_fuzzer found by usersim. The verifier_fuzzer called an API that allocated memory using ebpf_allocate() but verifier_fuzzer then called free()
Build ebpfcore_usersim.dll using the same files as used for ebpfcore.sys.
Add a test case that exercises the DriverEntry/DriverUnload code paths.

Testing

Tests will be expanded in the future once IOCTL support is added to usersim.

Documentation

No impact.

Installation

Installer files are updated in this PR.

dthaler · 2023-08-14T15:36:14Z

Blocked on iovisor/ubpf#335

codecov · 2023-08-14T18:39:33Z

Codecov Report

Merging #2744 (0b18e21) into main (000aed1) will decrease coverage by 1.21%.
The diff coverage is 82.35%.

@@            Coverage Diff             @@
##             main    #2744      +/-   ##
==========================================
- Coverage   87.23%   86.02%   -1.21%     
==========================================
  Files         129      137       +8     
  Lines       25314    25845     +531     
==========================================
+ Hits        22082    22234     +152     
- Misses       3232     3611     +379

Files Changed	Coverage Δ
libs/platform/user/framework.h	`100.00% <ø> (ø)`
tests/end_to_end/end_to_end.cpp	`97.90% <ø> (ø)`
libs/api/libbpf_program.cpp	`83.24% <66.66%> (-0.75%)`	⬇️
libs/api/libbpf_link.cpp	`94.44% <100.00%> (ø)`
libs/platform/ebpf_platform.c	`90.41% <100.00%> (-0.41%)`	⬇️
tests/end_to_end/test_helper.cpp	`87.00% <100.00%> (ø)`
tests/unit/ebpfcore_test.cpp	`100.00% <100.00%> (ø)`

... and 15 files with indirect coverage changes

saxena-anurag · 2023-08-16T04:24:20Z

libs/api/libbpf_link.cpp

@@ -15,7 +15,7 @@ bpf_link__pin(struct bpf_link* link, const char* path)
        return libbpf_err(-EBUSY);
    }

-    link->pin_path = strdup(path);
+    link->pin_path = ebpf_strdup(path);


strdup() internally calls malloc(). Before the usersim changes, the user mode implementation of ebpf_free() called free(), which was correct, as it matched with the corresponding malloc() in strdup().

Now it looks like there is only one implementation of ebpf_free(), both for user mode and kernel mode, and it calls ExFreePool(). So, for user mode code also, we are invoking user sim implementation of ExFreePool(). So 2 levels of indirection.

Is this expected?

Until #2677 is done, yes this is expected.

saxena-anurag · 2023-08-17T19:33:07Z

ebpfsvc/rpc_api.cpp

+    if (ebpf_logs) {
+        // The ebpf_logs buffer was allocated by the ebpf allocator whereas we
+        // must return a string allocated by the MIDL allocator.
+        *logs = (char*)MIDL_user_allocate(ebpf_logs_size);


It looks like there are 2 versions of MIDL_user_allocate() currently in the repo. The client version is calling ebpf_allocate(), whereas the server version is calling malloc().

If we change the implementation of the server version to also call ebpf_allocate(), then do we still need this change?

saxena-anurag · 2023-08-17T19:43:10Z

libs/api/libbpf_program.cpp

@@ -390,7 +399,12 @@ bpf_object__pin_programs(struct bpf_object* obj, const char* path)
        char buf[PATH_MAX];
        int len;

-        len = snprintf(buf, PATH_MAX, "%s/%s", path, __bpf_program__pin_name(prog));
+        char* pin_name = __bpf_program__pin_name(prog);


unrelated to changes in this PR: we are allocating memory in failure path. Instead should we instead save the pin paths generated when pinning the programs, and use the same in the failure case?

Signed-off-by: Dave Thaler <[email protected]>

Found my latest usersim code Signed-off-by: Dave Thaler <[email protected]>

Signed-off-by: Dave Thaler <[email protected]>

The verifier_fuzzer used ebpf_allocate() but then called free() Signed-off-by: Dave Thaler <[email protected]>

Signed-off-by: Dave Thaler <[email protected]>

dthaler requested review from poornagmsft, Alan-Jowett, saxena-anurag, shankarseal, dv-msft, delaramamiri, gtrevi and shpalani as code owners August 9, 2023 22:59

dthaler marked this pull request as draft August 9, 2023 22:59

dthaler force-pushed the ebpfcore-dll branch 2 times, most recently from a5b09d9 to d6601c2 Compare August 10, 2023 21:52

dthaler added blocked Blocked on another issue that must be done first and removed blocked Blocked on another issue that must be done first labels Aug 12, 2023

dthaler force-pushed the ebpfcore-dll branch 4 times, most recently from e6028cc to 0cd0d78 Compare August 13, 2023 20:32

dthaler added the blocked Blocked on another issue that must be done first label Aug 14, 2023

dthaler force-pushed the ebpfcore-dll branch from 4ea34dd to 0b18e21 Compare August 14, 2023 17:46

dthaler force-pushed the ebpfcore-dll branch from d945651 to 24ba7c9 Compare August 15, 2023 23:46

dthaler marked this pull request as ready for review August 16, 2023 01:04

saxena-anurag reviewed Aug 16, 2023

View reviewed changes

dthaler mentioned this pull request Aug 16, 2023

Bump external/usersim from 6d6f6e2 to b8f6883 #2754

Closed

dthaler removed the blocked Blocked on another issue that must be done first label Aug 17, 2023

dthaler force-pushed the ebpfcore-dll branch from 24ba7c9 to e00a73e Compare August 17, 2023 16:33

dthaler requested a review from saxena-anurag August 17, 2023 16:33

saxena-anurag reviewed Aug 17, 2023

View reviewed changes

saxena-anurag previously approved these changes Aug 17, 2023

View reviewed changes

dthaler added 13 commits August 17, 2023 14:59

Update to use latest usersim

6f0c9dc

Signed-off-by: Dave Thaler <[email protected]>

build ebpfcore under usersim

03e9d88

Signed-off-by: Dave Thaler <[email protected]>

Update usersim to latest

d17fc3e

Signed-off-by: Dave Thaler <[email protected]>

Fix compilation

853daba

Signed-off-by: Dave Thaler <[email protected]>

Work around analysis warnings

6f00a28

Signed-off-by: Dave Thaler <[email protected]>

Fix installer project

5f6c32e

Signed-off-by: Dave Thaler <[email protected]>

Fix memory issues in libbpf implementation

9a59a3a

Found my latest usersim code Signed-off-by: Dave Thaler <[email protected]>

Update usersim to latest

f310287

Signed-off-by: Dave Thaler <[email protected]>

Fix mismatched calloc/free function in ubpf_user.c

4b42e83

Signed-off-by: Dave Thaler <[email protected]>

Fix mismatched alloc/free in verifier_fuzzer

4b014d7

The verifier_fuzzer used ebpf_allocate() but then called free() Signed-off-by: Dave Thaler <[email protected]>

Temporarily use ubpf branch with memory fix

acf38ea

Signed-off-by: Dave Thaler <[email protected]>

Fix memory issues in ebpfsvc

c594542

Signed-off-by: Dave Thaler <[email protected]>

PR feedback from Anurag

b47cad1

Signed-off-by: Dave Thaler <[email protected]>

dthaler dismissed saxena-anurag’s stale review via b47cad1 August 17, 2023 22:01

dthaler force-pushed the ebpfcore-dll branch from e7b132d to b47cad1 Compare August 17, 2023 22:01

saxena-anurag approved these changes Aug 17, 2023

View reviewed changes

Alan-Jowett approved these changes Aug 17, 2023

View reviewed changes

dthaler enabled auto-merge August 17, 2023 22:05

dthaler added this pull request to the merge queue Aug 17, 2023

Merged via the queue into microsoft:main with commit 4e6bed3 Aug 17, 2023
66 checks passed

dthaler deleted the ebpfcore-dll branch August 17, 2023 23:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build ebpfcore as a DLL for testing #2744

Build ebpfcore as a DLL for testing #2744

dthaler commented Aug 9, 2023 •

edited

Loading

dthaler commented Aug 14, 2023

codecov bot commented Aug 14, 2023

saxena-anurag Aug 16, 2023 •

edited

Loading

dthaler Aug 16, 2023

saxena-anurag Aug 17, 2023 •

edited

Loading

dthaler Aug 17, 2023

saxena-anurag Aug 17, 2023

Build ebpfcore as a DLL for testing #2744

Build ebpfcore as a DLL for testing #2744

Conversation

dthaler commented Aug 9, 2023 • edited Loading

Description

Testing

Documentation

Installation

dthaler commented Aug 14, 2023

codecov bot commented Aug 14, 2023

Codecov Report

saxena-anurag Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

dthaler Aug 16, 2023

Choose a reason for hiding this comment

saxena-anurag Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

dthaler Aug 17, 2023

Choose a reason for hiding this comment

saxena-anurag Aug 17, 2023

Choose a reason for hiding this comment

dthaler commented Aug 9, 2023 •

edited

Loading

saxena-anurag Aug 16, 2023 •

edited

Loading

saxena-anurag Aug 17, 2023 •

edited

Loading