As currently implemented, it is not safe to open (and mmap) the
same kernel -> userspace ringbuffer multiple times.
Note: it's likely, that as implemented, even a single instance
is not thread safe without external locking.
Test: TreeHugger, atest bpf_existence_test bpf_module_test bpf-time-in-state-tests libbpf_android_test
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Iea6908746a58e7412c0ce89da29a764aefeaef47
Maciej Żenczykowskicommited 2024-06-15 03:58:24 -07:00
the clat verifyPerms() checker only wants to check the
existence and accessibility of the maps.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I3079ab316bcd88370d774fec5e88c1b5ff438266
Maciej Żenczykowskicommited 2024-06-15 03:54:09 -07:00
Per commit description of https://android-review.git.corp.google.com/c/platform/packages/modules/Connectivity/+/3109108 "Use SingleWriterBpfMap in BpfNetMaps":
- firewall.cpp writes to sConfigurationMap, sUidOwnerMap, and
sDataSaverEnabledMap, but that code only runs in tests because
it requires root privileges.
So either those maps need to not use SingleWriterBpfMap
in the BpfNetMaps (which would affect performance),
or we need to prevent the tests from grabbing the locks,
or the tests will (presumably) fail.
This avoids grabbing the locks from the test, even though
this is absolutely terrible and likely breaks things...
But in general the entire firewall.cpp file (and libconnectivity_native_test_utils target) are just one
large modularization violation in need of major escalation...
Test via:
atest resolv_{gold,stress,integration,unit}_test
Without this patch:
resolv_gold_test: Passed: 16
resolv_integration_test: Passed: 53, Failed: 2, Ignored: 118, Assumption Failed: 0 (Completed With ERRORS)
resolv_stress_test: Passed: 2
resolv_unit_test: Passed: 124, Failed: 2
4 tests failed
--------------
ResolverMultinetworkTest#DnsWithVpn
PrivateDns/TransportParameterizedTest#BlockDnsQuery/DoT
CallbackTest#tagSocketCallback
ResolvGetAddrInfoTest#InvalidParameters
With this patch:
resolv_gold_test: Passed: 16
resolv_integration_test: Passed: 178
resolv_stress_test: Passed: 2
resolv_unit_test: Passed: 126
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ie97d015ef66c3864a23484e7ed190fe7d091af5c
Maciej Żenczykowskicommited 2024-06-14 17:35:29 -07:00
The java bpfmap fd cache has some implications:
- this won't protect against two SingleWriterBpfMap instantiations
for the same map (path) in the same process
- we don't release any locks on BpfMap destruction.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ibb0b1f57974c307ea5e0fa699ce8d598c59f456e
Maciej Żenczykowskicommited 2024-06-14 15:56:01 -07:00
Only the non-CRITICAL test@mainline.o is loaded from netbpfload
on T and U < QPR3, everything else is loaded by platform bpfloader
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ic07a57cf93851d3a1411b62c97d8ce057507360b
Maciej Żenczykowskicommited 2024-06-14 15:09:41 -07:00
Per inspection of:
https://source.corp.google.com/h/prodkernel/kernel/upstream/torvalds/linux-2.6/+/master:kernel/bpf/syscall.c;drc=2884dc7d08d98a89d8d65121524bb7533183a63a;l=5786
bpf system call returns -EINVAL on unknown operations:
static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
{
...
switch (cmd) {
...
case BPF_OBJ_GET_INFO_BY_FD:
err = bpf_obj_get_info_by_fd(&attr, uattr.user);
break;
..
default:
err = -EINVAL;
break;
}
return err;
}
Equivalent logic is present in ACK 4.9-Q:
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
This is what should thus be returned on 4.9-T devices.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I124686ca360513f45e17afba23efa87acf1ce480
Maciej Żenczykowskicommited 2024-06-14 14:25:27 -07:00
and add an accessor to grab an exclusive lock on a R/W map open
(such a map could be accessed with a write-through cache)
Note: we can't grab a flock as that would occupy the full inode,
and all bpfmaps are actually (currently) the *same* anonymous inode.
As such we actually grab a lock on a range (a single byte),
the offset being determined by the unique bpf map id.
We include some very simple, but sufficient, correctness tests
in the critical boot path: this is to prevent any surprises
caused by kernel implementation changes.
$ adb root && sleep 1 && adb wait-for-device shell grep OFDLCK /proc/locks
id: OFDLCK ADVISORY [WRITE|READ] pid blkmaj:min:inode min_offset max_offset
11: OFDLCK ADVISORY READ -1 00:0e:1048 36 36
14: OFDLCK ADVISORY READ -1 00:0e:1048 35 35
15: OFDLCK ADVISORY READ -1 00:0e:1048 41 41
16: OFDLCK ADVISORY READ -1 00:0e:1048 40 40
22: OFDLCK ADVISORY READ -1 00:0e:1048 24 24
23: OFDLCK ADVISORY READ -1 00:0e:1048 17 17
24: OFDLCK ADVISORY READ -1 00:0e:1048 16 16
25: OFDLCK ADVISORY READ -1 00:0e:1048 13 13
OFDLCK probably means 'Open File Descriptor LoCK' since an OFDLCK
is associated with (held by) the file descriptor and not a process/pid,
on the given (anonymous in this case) block device + inode.
Where READ=shared and WRITE=exclusive.
There are (as yet) no exclusive locks held post boot.
The pid field is unfortunately always -1 (and cannot be manually set).
The 00:0e:1048 (or at least the inode portion) is random
(likely depends on boot ordering)
The final two fields (min and max offset) are the bpf map id.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I208e3450da3fe4689ad5fd578539f401f25a4fef
Maciej Żenczykowskicommited 2024-06-14 10:51:25 +00:00
when the program is missing due to too old kernel.
Program presence is already enforced by CRITICAL annotation and bpfloader.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ic5f53636c833e65c611a7b8069f28c088c249158
Maciej Żenczykowskicommited 2024-04-26 22:48:30 +00:00
Note:
2 == BPFLOADER_S_VERSION
19 == BPFLOADER_T_VERSION
25 == BPFLOADER_OBJ_AT_VER_VERSION
33 == BPFLOADER_IGNORED_ON_VERSION
37 == BPFLOADER_U_VERSION [*]
41 == BPFLOADER_V_VERSION [**]
and currently we have:
42 == BPFLOADER_MAINLINE_VERSION
43 == BPFLOADER_MAINLINE_T_VERSION
44 == BPFLOADER_MAINLINE_U_VERSION
45 == BPFLOADER_MAINLINE_V_VERSION
[*] udc-dev/udc-release are actually 38, as 37 was some earlier
udc beta2 (2.1 ifirc), and beta3+ switched to 38
[**] 24Q{1,2}-release (ie. U QPR2+) are technically actually 41,
but their platform bpfloader no longer even looks in /apex/com.android.tethering/etc/bpf/...
so it doesn't really matter (although 24Q1's platform
netbpfload still does, however that is obsoleted by
this change)
Thus we need to (as minimum for NetBpfLoad):
BPFLOADER_T_VERSION -> BPFLOADER_MAINLINE_T_VERSION
BPFLOADER_IGNORED_ON_VERSION -> BPFLOADER_MAINLINE_U_VERSION
to cause these to load appropriately from mainline.
One would perhaps also expect (as minimum for NetBpfLoad):
BPFLOADER_OBJ_AT_VERSION -> BPFLOADER_MAINLINE_U_VERSION
but (as maximum for platform bpfloader):
BPFLOADER_OBJ_AT_VERSION -> BPFLOADER_MAINLINE_U_VERSION
However, since the goal is actually for those to load via mainline on T+,
(with the 'old' version of the .o's loading on S via platform bpfloader)
thus one actually needs (as minimum for NetBpfLoad):
BPFLOADER_OBJ_AT_VERSION -> BPFLOADER_MAINLINE_T_VERSION
and (as maximum for platform bpfloader):
BPFLOADER_OBJ_AT_VERSION -> BPFLOADER_T_VERSION
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I4b688692ffca0db7ead20cefd9e7e892ca92c70a
Maciej Żenczykowskicommited 2024-03-19 21:33:55 +00:00
This CL is created as a best effort to migrate test targets
to the new android ownership model. If you find incorrect or unnecessary
attribution in this CL, please create a separate CL to fix that.
For more details please refer to the link below,
<add g3 doc link>
Bug: 304529413
Test: N/A
Change-Id: I243f17b3f0ad7af9ffa15ca242456e36688733f5
Merged-In: I243f17b3f0ad7af9ffa15ca242456e36688733f5
we can't use java for this, because pre-U-QPR2 does not include:
https://android-review.googlesource.com/c/platform/system/sepolicy/+/2821590
system_server dontaudit key_socket getopt
so we'll spew lots of:
11-08 07:52:43.776 1469 1469 I auditd : type=1400 audit(0.0:4): avc: denied { getopt } for comm="system_server" scontext=u:r:system_server:s0 tcontext=u:r:system_server:s0 tclass=key_socket permissive=0
11-08 07:52:44.360 1469 1469 I auditd : type=1400 audit(0.0:5): avc: denied { getopt } for comm="NetworkStats" scontext=u:r:system_server:s0 tcontext=u:r:system_server:s0 tclass=key_socket permissive=0
11-08 07:52:44.508 1469 1469 I auditd : type=1400 audit(0.0:7): avc: denied { getopt } for comm="android.bg" scontext=u:r:system_server:s0 tcontext=u:r:system_server:s0 tclass=key_socket permissive=0
and the like.
This is due to Java's Os.close() in:
libcore/luni/src/main/java/libcore/io/BlockGuardOs.java;l=100
calling:
if (fd.isSocket$()) if (isLingerSocket(fd)) ...
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ib87fa5e8f0dfd9fbbccb25fb58a9fa78d6a07111
Maciej Żenczykowskicommited 2023-11-09 11:08:58 +00:00
Returning the number of the file descriptor isn't actually useful since
it doesn't really tell you anything once the program has terminated.
(most logs we look at are long after the fact)
However, it does involve a fair bit of string processing to generate
the errors. This is particularly an issue for things we constantly
call and sometimes expect to fail. For example it is normal for
getNextKey() to fail with ENOENT at the end of iteration...
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ib94037440b5fcaa34cc8aa01b165431efabeb377
Maciej Żenczykowskicommited 2023-11-08 18:36:42 -08:00
(mostly to improve code coverage)
We can always revert if it ever turns out this is useful,
but I'd prefer for cgroup attach to be a once-at-boot
thing and handled either from bpfloader or the netd updatable
which are both directly C++ code.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I79b5ca8a47388cb6b9189234942e74ab6056aab9
Maciej Żenczykowskicommited 2023-10-25 17:40:35 +00:00
Mainline no longer supports Q.
These files were recently moved into p/m/C as part of a refactor.
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I0f06ace921db6c79d63c0048bdb73d167ff606cf
Maciej Żenczykowskicommited 2023-09-19 23:22:58 +00:00
This leaks (what should be) an internal only fd.
There's still multiple uses of getMap() left in:
common/native/bpf_headers/BpfMapTest.cpp
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I87a36f3f935ba9148ae4456fc6bf14b0e04f6038
Maciej Żenczykowskicommited 2023-08-21 19:53:04 +00:00
Maciej Żenczykowski
commited 
Gerrit Code Review
commited 
Patrick Rohr
commited 
Aditya Choudhary
commited 
Tomasz Wasilczyk
commited 
Motomu Utsumi
commited