We missed two incorrect specifiers in the previous commit with this same
title.
We use the %d format specificier for uid_t, which maps to
__kernel_uid32_t, which is unsigned. [1] This is undefined behavior
which can lead to paths with negative UIDs when erroneously large
values are passed for uid:
E libprocessgroup: No such cgroup attribute: /sys/fs/cgroup/uid_-89846/cgroup.freeze
Fix it with %u.
[1] https://cs.android.com/search?q=typedef.*__kernel_uid32_t&ss=android%2Fplatform%2Fsuperproject%2Fmain
Change-Id: Ica04b03526bd2e156f026a2797fe9912b259cd9f
The newer libc++ marks std::async with [[nodiscard]] in C++20 mode.
Bug: b/175635923
Test: treehugger
Change-Id: Ib63259983d952b97cf1b1c6c243c831cb72f9724
We only need a single binder thread to receive notifications, so we
should set the thread pool max to 0 rather than 1. startThreadPool()
starts one thread + the max count.
Test: m storageproxyd
Bug: 281951047
Fixes: 292030372
Change-Id: I53a90eaa2aa69469fd3a00b6da0d7061318c8ba9
Inline some trivial helpers and remove unused header declarations.
Remove fs_mgr_*access() as it is really just access().
Remove fs_mgr_overlayfs_super_device() as we always want the primary
slot and having this wrapper isn't particularly helpful.
Test: adb-remount-test
Change-Id: I2581fd7c7d5071cbb97778535b7811dbcb80d76e
According to https://abseil.io/tips/140, string constants should be
constexpr char array or string_view object. This avoids subtle bugs due
to the toolchain shuffling object initialization order between/within
compilation units.
string_view has bad interoperability between C APIs as many of those
functions require string values to be null-terminated. Thus we can only
rely on good old c-string constants.
This change groups all string constants together and change them all to
constexpr char array for consistent style.
Also remove some duplicated method definition.
Test: adb-remount-test
Change-Id: I1ed57e6dc24ce3750e72c5538c388a6872cd2b40
We use the %d format specificier for uid_t, which maps to
__kernel_uid32_t, which is unsigned. [1] This is undefined behavior
which can lead to paths with negative UIDs when erroneously large
values are passed for uid:
E libprocessgroup: No such cgroup attribute: /sys/fs/cgroup/uid_-89846/cgroup.freeze
Fix it with %u.
[1] https://cs.android.com/search?q=typedef.*__kernel_uid32_t&ss=android%2Fplatform%2Fsuperproject%2Fmain
Change-Id: Ibb52ba2503e30e2f20770b7d23629167e38d076a
When a read request is for a single sector, and the sector is
block-aligned, it falls into the ReadAlignedSector path, which assumes
block-sized reads. Fix this by clamping the expected size.
Bug: 291862304
Test: manual test of ReadWorker::ReadAlignedSector
full OTA
Change-Id: I00e460c333e8a9a4dc2433443e3633f3d794da1d
The Trusty storage proxy requires that the suspend service is started to
acquire a wakelock for UFS RPMB operations. Without the binder thread
pool running, starting this service results in at least a 1s polling
delay. This change ensures that we start the thread pool before handling
any RPMB operations, so acquiring the wakelock will complete as soon as
the service is ready without needing to poll once per second.
Test: m storageproxyd
Test: Artificially delay suspend_service to check if we poll
Bug: 281951047
Change-Id: I1a4cdd48d57201b0cf9c24523d22e5bdbcea376a
VFIO nodes, both the container (`vfio`) node and group (numbered)
nodes, should be located in `/dev/vfio`. This change prevents
ueventd from flattening that structure.
Test: Bind a device to VFIO driver to create a VFIO group
Change-Id: I635e9febe6bb52718df263e735479f361eacad4c
Since Android 14, Android does not use fsverity builtin signatures.
(fsverity remains supported, but signatures are verified in userspace,
or fsverity is used for integrity-only use cases.) Therefore, the only
reason to still run 'fsverity_init --load-verified-keys' at boot time is
to ensure that old files can still be opened, if:
- They were created by Android 13 or earlier, with an fsverity builtin
signature by a key in /{system,product}/etc/security/fsverity/.
- *And*, the kernel still has CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y.
However, it appears that this isn't actually needed anymore. Only two
features could potentially be affected: APK verity and updatable fonts.
APK verity wasn't widely rolled out before being disabled, and updatable
fonts have recovery logic in place for when the files cannot be opened.
And in any case, disabling CONFIG_FS_VERITY_BUILTIN_SIGNATURES in the
kernel is recommended and would avoid any problem.
Bug: 290064770
Test: presubmit
Change-Id: I3376c3f0b4b9bd4ba2fd614259522be0c1daafb6
Bug: 290835996
Test: libutils_fuzz_string8 for several minutes
String8::removeAll() has 2 serious problems:
1. When `other` is an empty string, `removeAll()` will loop infinitely
due to below process:
a) with `other` being empty string `""`, find() will call strstr()
on an empty string, which always returns `mString`, and thus
find() always return 0 in this case
b) with find() returns 0 for empty string, the next while loop in
String8::removeAll() will keep loop infinitely as `index` will
always be 0
This CL fixes this problem by returning true if `other` is an empty
string (i.e. `strlen(other) == 0`), this follows the logic that an
empty string will always be found and no actual remove needs to be
done.
2. When `other` is a NULL string, strstr() has undefined behavior. See
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf.
This undefined behavior on Android unfortunately causes immediate
segmentation fault as the current `strstr` implementation in bionic
libc doesn't check `needle` being NULL, and an access to a NULL
location is performed to check if the `needle` string is an empty
string, and thus causes segmentation fault.
This CL gives an error message and aborts instead of having a
segfault, and to keep some backward compatibility.
This CL also adds test for String8::removeAll()
Change-Id: Ie2ccee6767efe0fed476db4ec6072717198279e9
adding --help documentation to fastboot for --disable-super-optimization
and --disable-fastboot-info
Test: fastboot -h
Change-Id: Ia8993b3894d302a63cc97796d66e0af3fb004eef
Currently all I/O is processed through BufferSink. However, the
allocation of buffers is implicit in low-level helper functions, which
have no knowledge of how much data will actually be sent. As a result,
"allocation of buffers" and "tracking of how many bytes were written" is
disjoint. This will make it very difficult to break dependence on
dm-user, without a much more complex API.
This patch refactors how BufferSink is used. First, GetPayloadBuffer has
been deprecated in favor of AcquireBuffer. AcquireBuffer performs both
allocation and write tracking. The number of bytes written is not
necessarily the number of bytes allocated, and the new API allows for
this.
The "Process" helpers now take in an explicit buffer, and their callers
are responsible for allocating a buffer. This allows for
ReadUnalignedSector to have proper buffer offset tracking.
Because write tracking is now accurate, no explicit size needs to be
passed to WriteDmUserPayload. It can simply read the current watermark
in BufferSink.
This patch also removes XorSink, since its dependence on BufferSink made
this change more difficult.
Bug: 288273605
Test: snapuserd_test
Change-Id: Id9ca5044f9c0386f351b250349793b6b276b01b7
One of the first ERROR messages in logcat of a normal boot of Cuttlefish
is from failure to open SEPolicy.zip. This condition is expected.
Therefore don't try to load SEPolicy.zip when it doesn't exist. This
replaces the following log messages:
0 0 I init : Error: Apex SEPolicy failed signature check
0 0 I init : Loading APEX Sepolicy from /system/etc/selinux/apex/SEPolicy.zip
0 0 E init : Failed to open package /system/etc/selinux/apex/SEPolicy.zip: No such file or directory
... with just:
0 0 I init : No APEX Sepolicy found
Change-Id: If3a77407c35130165df5782b9ef91912e8374dbf
* changes:
snapuserd: Move Process ops out of Worker.
snapuserd: Move more fields out of Worker.
snapuserd: Split more methods out of Worker.
snapuserd: Create a ReadWorker class.
snapuserd: Create a MergeWorker class.
After MapDmUserCow, there is no further calls in this function. Snapshot
is mapped; Hence, checking remaining time is not required as
it may return false if the time was exceeded. This would mean function returning false even though snapshots were successfully mapped.
Bug: 291288998
Test: OTA
Change-Id: I4fd8fa6ef90a48885392297f217361507268ba51
Signed-off-by: Akilesh Kailash <akailash@google.com>
fs_mgr_defaults gets used in lots of places, propagate
local_include_dirs to where it's needed so it doesn't cause problems on
modules that don't have an "includes" directory.
Bug: 291083311
Test: builds
Change-Id: Ief2f6362f584b99d3e2dc7dc1b46450609ad189d