diff options
46 files changed, 501 insertions, 142 deletions
diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc index 5267b0294c..dff4c44671 100644 --- a/cmds/atrace/atrace.rc +++ b/cmds/atrace/atrace.rc @@ -181,6 +181,8 @@ on late-init chmod 0666 /sys/kernel/tracing/events/clk/clk_enable/enable chmod 0666 /sys/kernel/debug/tracing/events/clk/clk_set_rate/enable chmod 0666 /sys/kernel/tracing/events/clk/clk_set_rate/enable + chmod 0666 /sys/kernel/debug/tracing/events/printk/console/enable + chmod 0666 /sys/kernel/tracing/events/printk/console/enable # disk chmod 0666 /sys/kernel/tracing/events/f2fs/f2fs_get_data_block/enable @@ -295,8 +297,18 @@ on late-init write /sys/kernel/debug/tracing/synthetic_events "rss_stat_throttled unsigned int mm_id; unsigned int curr; int member; long size" # allow creating event triggers - chmod 0666 /sys/kernel/debug/tracing/events/kmem/rss_stat/trigger chmod 0666 /sys/kernel/tracing/events/kmem/rss_stat/trigger + chmod 0666 /sys/kernel/debug/tracing/events/kmem/rss_stat/trigger + + # allow enabling rss_stat_throttled + chmod 0666 /sys/kernel/tracing/events/synthetic/rss_stat_throttled/enable + chmod 0666 /sys/kernel/debug/tracing/events/synthetic/rss_stat_throttled/enable + +on late-init && property:ro.boot.fastboot.boottrace=enabled + setprop debug.atrace.tags.enableflags 802922 + setprop persist.traced.enable 0 + write /sys/kernel/debug/tracing/tracing_on 1 + write /sys/kernel/tracing/tracing_on 1 # Only create the tracing instance if persist.mm_events.enabled # Attempting to remove the tracing instance after it has been created @@ -386,6 +398,103 @@ on post-fs-data && property:persist.mm_events.enabled=true chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu23/trace chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu23/trace +# Handle hyp tracing instance +on late-init && property:ro.boot.hypervisor.vm.supported=1 + +# Hypervisor tracing instance doesn't support changing trace_clock + chmod 0440 /sys/kernel/debug/tracing/hyp/trace_clock + chmod 0440 /sys/kernel/tracing/hyp/trace_clock + + chmod 0660 /sys/kernel/debug/tracing/hyp/buffer_size_kb + chmod 0660 /sys/kernel/tracing/hyp/buffer_size_kb + + chmod 0660 /sys/kernel/debug/tracing/hyp/tracing_on + chmod 0660 /sys/kernel/tracing/hyp/tracing_on + +# Tracing disabled by default + write /sys/kernel/debug/tracing/hyp/tracing_on 0 + write /sys/kernel/tracing/hyp/tracing_on 0 + +# Read and truncate the hyp trace. + chmod 0660 /sys/kernel/debug/tracing/hyp/trace + chmod 0660 /sys/kernel/tracing/hyp/trace + +# Read and truncate the per-CPU kernel trace. +# Cannot use wildcards in .rc files. Update this if there is a phone with +# TODO(b/249050813, ioffe): introduce per-cpu wildcard + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu0/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu0/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu1/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu1/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu2/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu2/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu3/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu3/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu4/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu4/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu5/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu5/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu6/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu6/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu7/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu7/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu8/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu8/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu9/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu9/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu10/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu10/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu11/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu11/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu12/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu12/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu13/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu13/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu14/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu14/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu15/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu15/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu16/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu16/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu17/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu17/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu18/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu18/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu19/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu19/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu20/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu20/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu21/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu21/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu22/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu22/trace + chmod 0660 /sys/kernel/debug/tracing/hyp/per_cpu/cpu23/trace + chmod 0660 /sys/kernel/tracing/hyp/per_cpu/cpu23/trace + + chmod 0440 /sys/kernel/debug/tracing/hyp/events/header_page + chmod 0440 /sys/kernel/tracing/hyp/events/header_page + +# Hyp events start here + +# hyp_enter event + chmod 0660 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_enter/enable + chmod 0660 /sys/kernel/tracing/hyp/events/hyp/hyp_enter/enable +# TODO(b/249050813): should this be handled in kernel? + chmod 0440 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_enter/format + chmod 0440 /sys/kernel/tracing/hyp/events/hyp/hyp_enter/format + chmod 0440 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_enter/id + chmod 0440 /sys/kernel/tracing/hyp/events/hyp/hyp_enter/id + +# hyp_exit event + chmod 0660 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_exit/enable + chmod 0660 /sys/kernel/tracing/hyp/events/hyp/hyp_exit/enable +# TODO(b/249050813): should this be handled in kernel? + chmod 0440 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_exit/format + chmod 0440 /sys/kernel/tracing/hyp/events/hyp/hyp_exit/format + chmod 0440 /sys/kernel/debug/tracing/hyp/events/hyp/hyp_exit/id + chmod 0440 /sys/kernel/tracing/hyp/events/hyp/hyp_exit/id + + on property:persist.debug.atrace.boottrace=1 start boottrace @@ -393,3 +502,10 @@ on property:persist.debug.atrace.boottrace=1 service boottrace /system/bin/atrace --async_start -f /data/misc/boottrace/categories disabled oneshot + +on property:sys.boot_completed=1 && property:ro.boot.fastboot.boottrace=enabled + setprop debug.atrace.tags.enableflags 0 + setprop persist.traced.enable 1 + write /sys/kernel/debug/tracing/tracing_on 0 + write /sys/kernel/tracing/tracing_on 0 + diff --git a/cmds/dumpstate/OWNERS b/cmds/dumpstate/OWNERS index 5f56531754..ab81ecf4f4 100644 --- a/cmds/dumpstate/OWNERS +++ b/cmds/dumpstate/OWNERS @@ -3,3 +3,4 @@ set noparent gavincorkery@google.com nandana@google.com jsharkey@android.com +smoreland@google.com
\ No newline at end of file diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 34ea7597b4..ce3d669a63 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -442,6 +442,16 @@ static unique_fd open_current_profile(uid_t uid, userid_t user, const std::strin static unique_fd open_reference_profile(uid_t uid, const std::string& package_name, const std::string& location, bool read_write, bool is_secondary_dex) { std::string profile = create_reference_profile_path(package_name, location, is_secondary_dex); + if (read_write && GetBoolProperty("dalvik.vm.useartservice", false)) { + // ART Service doesn't use flock and instead assumes profile files are + // immutable, so ensure we don't open a file for writing when it's + // active. + // TODO(b/251921228): Normally installd isn't called at all in that + // case, but OTA is still an exception that uses the legacy code. + LOG(ERROR) << "Opening ref profile " << profile + << " for writing is unsafe when ART Service is enabled."; + return invalid_unique_fd(); + } return open_profile( uid, profile, @@ -450,14 +460,13 @@ static unique_fd open_reference_profile(uid_t uid, const std::string& package_na } static UniqueFile open_reference_profile_as_unique_file(uid_t uid, const std::string& package_name, - const std::string& location, bool read_write, bool is_secondary_dex) { + const std::string& location, + bool is_secondary_dex) { std::string profile_path = create_reference_profile_path(package_name, location, is_secondary_dex); - unique_fd ufd = open_profile( - uid, - profile_path, - read_write ? (O_CREAT | O_RDWR) : O_RDONLY, - S_IRUSR | S_IWUSR | S_IRGRP); // so that ART can also read it when apps run. + unique_fd ufd = open_profile(uid, profile_path, O_RDONLY, + S_IRUSR | S_IWUSR | + S_IRGRP); // so that ART can also read it when apps run. return UniqueFile(ufd.release(), profile_path, [](const std::string& path) { clear_profile(path); @@ -1104,8 +1113,7 @@ UniqueFile maybe_open_reference_profile(const std::string& pkgname, location = profile_name; } } - return open_reference_profile_as_unique_file(uid, pkgname, location, /*read_write*/false, - is_secondary_dex); + return open_reference_profile_as_unique_file(uid, pkgname, location, is_secondary_dex); } // Opens the vdex files and assigns the input fd to in_vdex_wrapper and the output fd to diff --git a/cmds/service/service.cpp b/cmds/service/service.cpp index d5ca725eb9..5e8ef5d7d8 100644 --- a/cmds/service/service.cpp +++ b/cmds/service/service.cpp @@ -75,7 +75,7 @@ int main(int argc, char* const argv[]) ProcessState::initWithDriver("/dev/vndbinder"); #endif #ifndef __ANDROID__ - setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1})); + setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1})); #endif sp<IServiceManager> sm = defaultServiceManager(); fflush(stdout); diff --git a/include/android/OWNERS b/include/android/OWNERS new file mode 100644 index 0000000000..38f9c5563a --- /dev/null +++ b/include/android/OWNERS @@ -0,0 +1 @@ +per-file input.h, keycodes.h = file:platform/frameworks/base:/INPUT_OWNERS diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index aec3080b26..baeb565b5e 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -74,9 +74,6 @@ cc_defaults { name: "libbinder_common_defaults", host_supported: true, - // for vndbinder and binderRpcTest - vendor_available: true, - srcs: [ "Binder.cpp", "BpBinder.cpp", @@ -200,7 +197,6 @@ cc_defaults { cc_library_headers { name: "trusty_mock_headers", - vendor_available: true, host_supported: true, export_include_dirs: [ @@ -215,7 +211,6 @@ cc_library_headers { cc_defaults { name: "trusty_mock_defaults", - vendor_available: true, host_supported: true, header_libs: [ @@ -309,6 +304,8 @@ cc_library { version_script: "libbinder.map", + // for vndbinder + vendor_available: true, vndk: { enabled: true, }, @@ -471,7 +468,6 @@ cc_library_shared { cc_library_static { name: "libbinder_tls_static", defaults: ["libbinder_tls_defaults"], - vendor_available: true, visibility: [ ":__subpackages__", ], @@ -556,6 +552,7 @@ cc_library { "//packages/modules/Virtualization/javalib/jni", "//packages/modules/Virtualization/vm_payload", "//device/google/cuttlefish/shared/minidroid:__subpackages__", + "//system/software_defined_vehicle:__subpackages__", ], } diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp index 2e7030437c..51b971651d 100644 --- a/libs/binder/RecordedTransaction.cpp +++ b/libs/binder/RecordedTransaction.cpp @@ -16,6 +16,7 @@ #include <android-base/file.h> #include <android-base/logging.h> +#include <android-base/scopeguard.h> #include <android-base/unique_fd.h> #include <binder/RecordedTransaction.h> #include <sys/mman.h> @@ -176,13 +177,33 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd RecordedTransaction t; ChunkDescriptor chunk; const long pageSize = sysconf(_SC_PAGE_SIZE); + struct stat fileStat; + if (fstat(fd.get(), &fileStat) != 0) { + LOG(ERROR) << "Unable to get file information"; + return std::nullopt; + } + + off_t fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR); + if (fdCurrentPosition == -1) { + LOG(ERROR) << "Invalid offset in file descriptor."; + return std::nullopt; + } do { + if (fileStat.st_size < (fdCurrentPosition + (off_t)sizeof(ChunkDescriptor))) { + LOG(ERROR) << "Not enough file remains to contain expected chunk descriptor"; + return std::nullopt; + } transaction_checksum_t checksum = 0; if (NO_ERROR != readChunkDescriptor(fd, &chunk, &checksum)) { LOG(ERROR) << "Failed to read chunk descriptor."; return std::nullopt; } - off_t fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR); + + fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR); + if (fdCurrentPosition == -1) { + LOG(ERROR) << "Invalid offset in file descriptor."; + return std::nullopt; + } off_t mmapPageAlignedStart = (fdCurrentPosition / pageSize) * pageSize; off_t mmapPayloadStartOffset = fdCurrentPosition - mmapPageAlignedStart; @@ -194,14 +215,24 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd size_t chunkPayloadSize = chunk.dataSize + PADDING8(chunk.dataSize) + sizeof(transaction_checksum_t); + if (chunkPayloadSize > (size_t)(fileStat.st_size - fdCurrentPosition)) { + LOG(ERROR) << "Chunk payload exceeds remaining file size."; + return std::nullopt; + } + if (PADDING8(chunkPayloadSize) != 0) { LOG(ERROR) << "Invalid chunk size, not aligned " << chunkPayloadSize; return std::nullopt; } - transaction_checksum_t* payloadMap = reinterpret_cast<transaction_checksum_t*>( - mmap(NULL, chunkPayloadSize + mmapPayloadStartOffset, PROT_READ, MAP_SHARED, - fd.get(), mmapPageAlignedStart)); + size_t memoryMappedSize = chunkPayloadSize + mmapPayloadStartOffset; + void* mappedMemory = + mmap(NULL, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), mmapPageAlignedStart); + auto mmap_guard = android::base::make_scope_guard( + [mappedMemory, memoryMappedSize] { munmap(mappedMemory, memoryMappedSize); }); + + transaction_checksum_t* payloadMap = + reinterpret_cast<transaction_checksum_t*>(mappedMemory); payloadMap += mmapPayloadStartOffset / sizeof(transaction_checksum_t); // Skip chunk descriptor and required mmap // page-alignment @@ -218,7 +249,12 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd LOG(ERROR) << "Checksum failed."; return std::nullopt; } - lseek(fd.get(), chunkPayloadSize, SEEK_CUR); + + fdCurrentPosition = lseek(fd.get(), chunkPayloadSize, SEEK_CUR); + if (fdCurrentPosition == -1) { + LOG(ERROR) << "Invalid offset in file descriptor."; + return std::nullopt; + } switch (chunk.chunkType) { case HEADER_CHUNK: { @@ -255,7 +291,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd break; default: LOG(INFO) << "Unrecognized chunk."; - continue; + break; } } while (chunk.chunkType != END_CHUNK); diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index ce6ef2becf..1a821f14c9 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -90,7 +90,7 @@ size_t RpcSession::getMaxIncomingThreads() { return mMaxIncomingThreads; } -void RpcSession::setMaxOutgoingThreads(size_t threads) { +void RpcSession::setMaxOutgoingConnections(size_t threads) { RpcMutexLockGuard _l(mMutex); LOG_ALWAYS_FATAL_IF(mStartedSetup, "Must set max outgoing threads before setting up connections"); @@ -932,7 +932,8 @@ status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, Co (session->server() ? "This is a server session, so see RpcSession::setMaxIncomingThreads " "for the corresponding client" - : "This is a client session, so see RpcSession::setMaxOutgoingThreads " + : "This is a client session, so see " + "RpcSession::setMaxOutgoingConnections " "for this client or RpcServer::setMaxThreads for the corresponding " "server")); return WOULD_BLOCK; diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp index b27f1028d4..2b0e5bae0d 100644 --- a/libs/binder/RpcState.cpp +++ b/libs/binder/RpcState.cpp @@ -557,13 +557,12 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti .parcelDataSize = static_cast<uint32_t>(data.dataSize()), }; - constexpr size_t kWaitMaxUs = 1000000; - constexpr size_t kWaitLogUs = 10000; - size_t waitUs = 0; - // Oneway calls have no sync point, so if many are sent before, whether this // is a twoway or oneway transaction, they may have filled up the socket. // So, make sure we drain them before polling + constexpr size_t kWaitMaxUs = 1000000; + constexpr size_t kWaitLogUs = 10000; + size_t waitUs = 0; iovec iovs[]{ {&command, sizeof(RpcWireHeader)}, @@ -591,8 +590,9 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti }, rpcFields->mFds.get()); status != OK) { - // TODO(b/167966510): need to undo onBinderLeaving - we know the - // refcount isn't successfully transferred. + // rpcSend calls shutdownAndWait, so all refcounts should be reset. If we ever tolerate + // errors here, then we may need to undo the binder-sent counts for the transaction as + // well as for the binder objects in the Parcel return status; } diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp index 194254ac69..2b67f030e0 100644 --- a/libs/binder/ServiceManagerHost.cpp +++ b/libs/binder/ServiceManagerHost.cpp @@ -159,8 +159,8 @@ sp<IBinder> getDeviceService(std::vector<std::string>&& serviceDispatcherArgs, LOG_ALWAYS_FATAL_IF(!forwardResult->hostPort().has_value()); auto rpcSession = RpcSession::make(); - if (options.maxOutgoingThreads.has_value()) { - rpcSession->setMaxOutgoingThreads(*options.maxOutgoingThreads); + if (options.maxOutgoingConnections.has_value()) { + rpcSession->setMaxOutgoingConnections(*options.maxOutgoingConnections); } if (status_t status = rpcSession->setupInetClient("127.0.0.1", *forwardResult->hostPort()); diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index c78f870153..55167a7db0 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -224,12 +224,12 @@ bool checkPermission(const String16& permission, pid_t pid, uid_t uid, // } // Resources are cleaned up when the object is destroyed. // -// For each returned binder object, at most |maxOutgoingThreads| outgoing threads are instantiated. -// Hence, only |maxOutgoingThreads| calls can be made simultaneously. Additional calls are blocked -// if there are |maxOutgoingThreads| ongoing calls. See RpcSession::setMaxOutgoingThreads. -// If |maxOutgoingThreads| is not set, default is |RpcSession::kDefaultMaxOutgoingThreads|. +// For each returned binder object, at most |maxOutgoingConnections| outgoing connections are +// instantiated, depending on how many the service on the device is configured with. +// Hence, only |maxOutgoingConnections| calls can be made simultaneously. +// See also RpcSession::setMaxOutgoingConnections. struct RpcDelegateServiceManagerOptions { - std::optional<size_t> maxOutgoingThreads; + std::optional<size_t> maxOutgoingConnections; }; sp<IServiceManager> createRpcDelegateServiceManager( const RpcDelegateServiceManagerOptions& options); diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h index 25193a3428..1001b64ede 100644 --- a/libs/binder/include/binder/RpcServer.h +++ b/libs/binder/include/binder/RpcServer.h @@ -119,7 +119,10 @@ public: [[nodiscard]] status_t setupExternalServer(base::unique_fd serverFd); /** - * This must be called before adding a client session. + * This must be called before adding a client session. This corresponds + * to the number of incoming connections to RpcSession objects in the + * server, which will correspond to the number of outgoing connections + * in client RpcSession objects. * * If this is not specified, this will be a single-threaded server. * diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h index 40faf2c3b1..e301569acc 100644 --- a/libs/binder/include/binder/RpcSession.h +++ b/libs/binder/include/binder/RpcSession.h @@ -67,26 +67,30 @@ public: /** * Set the maximum number of incoming threads allowed to be made (for things like callbacks). * By default, this is 0. This must be called before setting up this connection as a client. - * Server sessions will inherits this value from RpcServer. + * Server sessions will inherits this value from RpcServer. Each thread will serve a + * connection to the remote RpcSession. * * If this is called, 'shutdown' on this session must also be called. * Otherwise, a threadpool will leak. * - * TODO(b/189955605): start these dynamically + * TODO(b/189955605): start these lazily - currently all are started */ void setMaxIncomingThreads(size_t threads); size_t getMaxIncomingThreads(); /** - * Set the maximum number of outgoing threads allowed to be made. + * Set the maximum number of outgoing connections allowed to be made. * By default, this is |kDefaultMaxOutgoingThreads|. This must be called before setting up this * connection as a client. * - * This limits the number of outgoing threads on top of the remote peer setting. This RpcSession - * will only instantiate |min(maxOutgoingThreads, remoteMaxThreads)| outgoing threads, where - * |remoteMaxThreads| can be retrieved from the remote peer via |getRemoteMaxThreads()|. + * For an RpcSession client, if you are connecting to a server which starts N threads, + * then this must be set to >= N. If you set the maximum number of outgoing connections + * to 1, but the server requests 10, then it would be considered an error. If you set a + * maximum number of connections to 10, and the server requests 1, then only 1 will be + * created. This API is used to limit the amount of resources a server can request you + * create. */ - void setMaxOutgoingThreads(size_t threads); + void setMaxOutgoingConnections(size_t threads); size_t getMaxOutgoingThreads(); /** diff --git a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp index 42d226b805..e273dff5f6 100644 --- a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp +++ b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp @@ -126,11 +126,11 @@ AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* session, void ARpcSession_setFileDescriptorTransportMode(ARpcSession* session, ARpcSession_FileDescriptorTransportMode mode); -// Sets the maximum number of incoming threads. +// Sets the maximum number of incoming threads, to service connections. void ARpcSession_setMaxIncomingThreads(ARpcSession* session, size_t threads); -// Sets the maximum number of outgoing threads. -void ARpcSession_setMaxOutgoingThreads(ARpcSession* session, size_t threads); +// Sets the maximum number of outgoing connections. +void ARpcSession_setMaxOutgoingConnections(ARpcSession* session, size_t threads); // Decrements the refcount of the underlying RpcSession object. void ARpcSession_free(ARpcSession* session); diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp index daff8c1e16..971b3a056a 100644 --- a/libs/binder/libbinder_rpc_unstable.cpp +++ b/libs/binder/libbinder_rpc_unstable.cpp @@ -265,8 +265,8 @@ void ARpcSession_setMaxIncomingThreads(ARpcSession* handle, size_t threads) { session->setMaxIncomingThreads(threads); } -void ARpcSession_setMaxOutgoingThreads(ARpcSession* handle, size_t threads) { +void ARpcSession_setMaxOutgoingConnections(ARpcSession* handle, size_t threads) { auto session = handleToStrongPointer<RpcSession>(handle); - session->setMaxOutgoingThreads(threads); + session->setMaxOutgoingConnections(threads); } } diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h index 86d5ed27b8..43159d8ba2 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -22,6 +22,16 @@ __BEGIN_DECLS +enum AServiceManager_AddServiceFlag : uint32_t { + /** + * This allows processes with AID_ISOLATED to get the binder of the service added. + * + * Services with methods that perform file IO, web socket creation or ways to egress data must + * not be added with this flag for privacy concerns. + */ + ADD_SERVICE_ALLOW_ISOLATED = 1, +}; + /** * This registers the service with the default service manager under this instance name. This does * not take ownership of binder. @@ -46,12 +56,13 @@ __attribute__((warn_unused_result)) binder_exception_t AServiceManager_addServic * * \param binder object to register globally with the service manager. * \param instance identifier of the service. This will be used to lookup the service. - * \param allowIsolated allows if this service can be isolated. + * \param flag an AServiceManager_AddServiceFlag enum to denote how the service should be added. * * \return EX_NONE on success. */ -__attribute__((warn_unused_result)) binder_exception_t AServiceManager_addServiceWithAllowIsolated( - AIBinder* binder, const char* instance, bool allowIsolated) __INTRODUCED_IN(34); +__attribute__((warn_unused_result)) binder_exception_t AServiceManager_addServiceWithFlag( + AIBinder* binder, const char* instance, const AServiceManager_AddServiceFlag flag) + __INTRODUCED_IN(34); /** * Gets a binder object with this specific instance name. Will return nullptr immediately if the diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt index 5f2f617946..1078fb2b16 100644 --- a/libs/binder/ndk/libbinder_ndk.map.txt +++ b/libs/binder/ndk/libbinder_ndk.map.txt @@ -158,12 +158,12 @@ LIBBINDER_NDK34 { # introduced=UpsideDownCake AServiceManager_getUpdatableApexName; # systemapi AServiceManager_registerForServiceNotifications; # systemapi llndk AServiceManager_NotificationRegistration_delete; # systemapi llndk + AServiceManager_addServiceWithFlag; # systemapi llndk }; LIBBINDER_NDK_PLATFORM { global: AParcel_getAllowFds; - AServiceManager_addServiceWithAllowIsolated; extern "C++" { AIBinder_fromPlatformBinder*; AIBinder_toPlatformBinder*; diff --git a/libs/binder/ndk/service_manager.cpp b/libs/binder/ndk/service_manager.cpp index 2763ddb622..84da459454 100644 --- a/libs/binder/ndk/service_manager.cpp +++ b/libs/binder/ndk/service_manager.cpp @@ -42,14 +42,15 @@ binder_exception_t AServiceManager_addService(AIBinder* binder, const char* inst return PruneException(exception); } -binder_exception_t AServiceManager_addServiceWithAllowIsolated(AIBinder* binder, - const char* instance, - bool allowIsolated) { +binder_exception_t AServiceManager_addServiceWithFlag(AIBinder* binder, const char* instance, + const AServiceManager_AddServiceFlag flag) { if (binder == nullptr || instance == nullptr) { return EX_ILLEGAL_ARGUMENT; } sp<IServiceManager> sm = defaultServiceManager(); + + bool allowIsolated = flag & AServiceManager_AddServiceFlag::ADD_SERVICE_ALLOW_ISOLATED; status_t exception = sm->addService(String16(instance), binder->getBinder(), allowIsolated); return PruneException(exception); } diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index 5b2532ab4e..882f1d66b7 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -52,7 +52,7 @@ constexpr char kLazyBinderNdkUnitTestService[] = "LazyBinderNdkUnitTest"; constexpr char kForcePersistNdkUnitTestService[] = "ForcePersistNdkUnitTestService"; constexpr char kActiveServicesNdkUnitTestService[] = "ActiveServicesNdkUnitTestService"; -constexpr unsigned int kShutdownWaitTime = 10; +constexpr unsigned int kShutdownWaitTime = 11; constexpr uint64_t kContextTestValue = 0xb4e42fb4d9a1d715; class MyTestFoo : public IFoo { diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp index afd414a7cb..d36ebac109 100644 --- a/libs/binder/rust/Android.bp +++ b/libs/binder/rust/Android.bp @@ -21,6 +21,7 @@ rust_library { ], host_supported: true, vendor_available: true, + product_available: true, target: { darwin: { enabled: false, @@ -72,6 +73,7 @@ rust_library { ], host_supported: true, vendor_available: true, + product_available: true, target: { darwin: { enabled: false, @@ -129,6 +131,7 @@ rust_bindgen { ], host_supported: true, vendor_available: true, + product_available: true, // Currently necessary for host builds // TODO(b/31559095): bionic on host should define this diff --git a/libs/binder/rust/rpcbinder/Android.bp b/libs/binder/rust/rpcbinder/Android.bp index da9797b25c..0067a20484 100644 --- a/libs/binder/rust/rpcbinder/Android.bp +++ b/libs/binder/rust/rpcbinder/Android.bp @@ -26,6 +26,7 @@ rust_library { visibility: [ "//device/google/cuttlefish/shared/minidroid/sample", "//packages/modules/Virtualization:__subpackages__", + "//system/software_defined_vehicle:__subpackages__", ], apex_available: [ "//apex_available:platform", diff --git a/libs/binder/rust/rpcbinder/src/session.rs b/libs/binder/rust/rpcbinder/src/session.rs index 0b517cf613..28c5390665 100644 --- a/libs/binder/rust/rpcbinder/src/session.rs +++ b/libs/binder/rust/rpcbinder/src/session.rs @@ -75,11 +75,14 @@ impl RpcSessionRef { }; } - /// Sets the maximum number of outgoing threads. - pub fn set_max_outgoing_threads(&self, threads: usize) { + /// Sets the maximum number of outgoing connections. + pub fn set_max_outgoing_connections(&self, connections: usize) { // SAFETY - Only passes the 'self' pointer as an opaque handle. unsafe { - binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingThreads(self.as_ptr(), threads) + binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections( + self.as_ptr(), + connections, + ) }; } diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs index 6f686fbd93..5557168055 100644 --- a/libs/binder/rust/src/native.rs +++ b/libs/binder/rust/src/native.rs @@ -209,8 +209,8 @@ impl<T: Remotable> Binder<T> { } /// Mark this binder object with local stability, which is vendor if we are - /// building for the VNDK and system otherwise. - #[cfg(any(vendor_ndk, android_vndk))] + /// building for android_vendor and system otherwise. + #[cfg(android_vendor)] fn mark_local_stability(&mut self) { unsafe { // Safety: Self always contains a valid `AIBinder` pointer, so @@ -220,8 +220,8 @@ impl<T: Remotable> Binder<T> { } /// Mark this binder object with local stability, which is vendor if we are - /// building for the VNDK and system otherwise. - #[cfg(not(any(vendor_ndk, android_vndk)))] + /// building for android_vendor and system otherwise. + #[cfg(not(android_vendor))] fn mark_local_stability(&mut self) { unsafe { // Safety: Self always contains a valid `AIBinder` pointer, so diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 2a4fc48af6..0f0d64aea8 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -138,7 +138,6 @@ cc_test { aidl_interface { name: "binderRpcTestIface", - vendor_available: true, host_supported: true, unstable: true, srcs: [ @@ -159,7 +158,6 @@ aidl_interface { cc_library_static { name: "libbinder_tls_test_utils", - vendor_available: true, host_supported: true, target: { darwin: { @@ -213,7 +211,6 @@ cc_defaults { defaults: [ "binderRpcTest_common_defaults", ], - vendor_available: true, gtest: false, auto_gen_config: false, srcs: [ @@ -224,18 +221,10 @@ cc_defaults { cc_defaults { name: "binderRpcTest_defaults", - vendor_available: true, target: { android: { test_suites: ["vts"], }, - - vendor: { - shared_libs: [ - "libbinder_trusty", - "libtrusty", - ], - }, }, defaults: [ "binderRpcTest_common_defaults", @@ -397,11 +386,6 @@ cc_binary { cc_test { name: "binderRpcTest", - // TODO(b/269799024) - test_options: { - unit_test: false, - }, - defaults: [ "binderRpcTest_defaults", "binderRpcTest_shared_defaults", diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp index 464da60dde..77a5fa8d65 100644 --- a/libs/binder/tests/binderHostDeviceTest.cpp +++ b/libs/binder/tests/binderHostDeviceTest.cpp @@ -66,7 +66,7 @@ MATCHER_P(StatusEq, expected, (negation ? "not " : "") + statusToString(expected void initHostRpcServiceManagerOnce() { static std::once_flag gSmOnce; std::call_once(gSmOnce, [] { - setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1})); + setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1})); }); } diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 955c650205..8974ad745d 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -507,7 +507,13 @@ TEST_F(BinderLibTest, Freeze) { } EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, true, 0)); - EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, true, 0)); + + // b/268232063 - succeeds ~0.08% of the time + { + auto ret = IPCThreadState::self()->freeze(pid, true, 0); + EXPECT_TRUE(ret == -EAGAIN || ret == OK); + } + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, true, 1000)); EXPECT_EQ(FAILED_TRANSACTION, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); @@ -1370,7 +1376,7 @@ TEST_F(BinderLibTest, ThreadPoolAvailableThreads) { })); } - data.writeInt32(100); + data.writeInt32(500); // Give a chance for all threads to be used EXPECT_THAT(server->transact(BINDER_LIB_TEST_UNLOCK_AFTER_MS, data, &reply), NO_ERROR); diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 84c93ddc30..6e34d254bb 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -350,7 +350,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( for (const auto& session : sessions) { CHECK(session->setProtocolVersion(clientVersion)); session->setMaxIncomingThreads(options.numIncomingConnections); - session->setMaxOutgoingThreads(options.numOutgoingConnections); + session->setMaxOutgoingConnections(options.numOutgoingConnections); session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode); switch (socketType) { @@ -544,6 +544,8 @@ TEST_P(BinderRpc, OnewayCallQueueingWithFds) { GTEST_SKIP() << "This test requires multiple threads"; } + constexpr size_t kNumServerThreads = 3; + // This test forces a oneway transaction to be queued by issuing two // `blockingSendFdOneway` calls, then drains the queue by issuing two // `blockingRecvFd` calls. @@ -552,7 +554,7 @@ TEST_P(BinderRpc, OnewayCallQueueingWithFds) { // https://developer.android.com/reference/android/os/IBinder#FLAG_ONEWAY auto proc = createRpcTestSocketServerProcess({ - .numThreads = 3, + .numThreads = kNumServerThreads, .clientFileDescriptorTransportMode = RpcSession::FileDescriptorTransportMode::UNIX, .serverSupportedFileDescriptorTransportModes = {RpcSession::FileDescriptorTransportMode::UNIX}, @@ -573,6 +575,8 @@ TEST_P(BinderRpc, OnewayCallQueueingWithFds) { EXPECT_OK(proc.rootIface->blockingRecvFd(&fdB)); CHECK(android::base::ReadFdToString(fdB.get(), &result)); EXPECT_EQ(result, "b"); + + saturateThreadPool(kNumServerThreads, proc.rootIface); } TEST_P(BinderRpc, OnewayCallQueueing) { diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp index 714f0636f9..a27bd2f2e6 100644 --- a/libs/binder/tests/binderRpcTestService.cpp +++ b/libs/binder/tests/binderRpcTestService.cpp @@ -85,7 +85,9 @@ public: } }; -int main(int argc, const char* argv[]) { +int main(int argc, char* argv[]) { + android::base::InitLogging(argv, android::base::StderrLogger, android::base::DefaultAborter); + LOG_ALWAYS_FATAL_IF(argc != 3, "Invalid number of arguments: %d", argc); base::unique_fd writeEnd(atoi(argv[1])); base::unique_fd readEnd(atoi(argv[2])); diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp index b3bb5ebda7..84abbac6c6 100644 --- a/libs/binder/tests/binderRpcTestTrusty.cpp +++ b/libs/binder/tests/binderRpcTestTrusty.cpp @@ -71,7 +71,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( auto session = android::RpcSession::make(std::move(factory)); EXPECT_TRUE(session->setProtocolVersion(clientVersion)); - session->setMaxOutgoingThreads(options.numOutgoingConnections); + session->setMaxOutgoingConnections(options.numOutgoingConnections); session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode); status = session->setupPreconnectedClient({}, [&]() { diff --git a/libs/binder/tests/unit_fuzzers/Android.bp b/libs/binder/tests/unit_fuzzers/Android.bp index 8ea948cc15..a88158299e 100644 --- a/libs/binder/tests/unit_fuzzers/Android.bp +++ b/libs/binder/tests/unit_fuzzers/Android.bp @@ -104,3 +104,42 @@ cc_fuzz { defaults: ["binder_fuzz_defaults"], srcs: ["MemoryDealerFuzz.cpp"], } + +cc_fuzz { + name: "binder_recordedTransactionFileFuzz", + defaults: ["binder_fuzz_defaults"], + srcs: ["RecordedTransactionFileFuzz.cpp"], + corpus: [ + "recorded_transaction_corpus/*", + ], +} + +cc_fuzz { + name: "binder_recordedTransactionFuzz", + defaults: ["binder_fuzz_defaults"], + srcs: ["RecordedTransactionFuzz.cpp"], + target: { + android: { + shared_libs: [ + "libcutils", + "libutils", + "libbase", + "libbinder", + ], + static_libs: ["libbinder_random_parcel"], + }, + host: { + static_libs: [ + "libcutils", + "liblog", + "libutils", + "libbase", + "libbinder", + "libbinder_random_parcel", + ], + }, + darwin: { + enabled: false, + }, + }, +} diff --git a/libs/binder/tests/unit_fuzzers/RecordedTransactionFileFuzz.cpp b/libs/binder/tests/unit_fuzzers/RecordedTransactionFileFuzz.cpp new file mode 100644 index 0000000000..73790fae49 --- /dev/null +++ b/libs/binder/tests/unit_fuzzers/RecordedTransactionFileFuzz.cpp @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <android-base/macros.h> +#include <binder/RecordedTransaction.h> +#include <filesystem> + +#include "fuzzer/FuzzedDataProvider.h" + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + std::FILE* intermediateFile = std::tmpfile(); + fwrite(data, sizeof(uint8_t), size, intermediateFile); + rewind(intermediateFile); + int fileNumber = fileno(intermediateFile); + + android::base::unique_fd fd(fileNumber); + + auto transaction = android::binder::debug::RecordedTransaction::fromFile(fd); + + std::fclose(intermediateFile); + + if (transaction.has_value()) { + intermediateFile = std::tmpfile(); + + android::base::unique_fd fdForWriting(fileno(intermediateFile)); + auto writeStatus ATTRIBUTE_UNUSED = transaction.value().dumpToFile(fdForWriting); + + std::fclose(intermediateFile); + } + + return 0; +} diff --git a/libs/binder/tests/unit_fuzzers/RecordedTransactionFuzz.cpp b/libs/binder/tests/unit_fuzzers/RecordedTransactionFuzz.cpp new file mode 100644 index 0000000000..943fb9f285 --- /dev/null +++ b/libs/binder/tests/unit_fuzzers/RecordedTransactionFuzz.cpp @@ -0,0 +1,64 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <android-base/macros.h> +#include <binder/RecordedTransaction.h> +#include <fuzzbinder/random_parcel.h> +#include <filesystem> +#include <string> + +#include "fuzzer/FuzzedDataProvider.h" + +using android::fillRandomParcel; + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + FuzzedDataProvider provider = FuzzedDataProvider(data, size); + + android::String16 interfaceName = + android::String16(provider.ConsumeRandomLengthString().c_str()); + + uint32_t code = provider.ConsumeIntegral<uint32_t>(); + uint32_t flags = provider.ConsumeIntegral<uint32_t>(); + time_t sec = provider.ConsumeIntegral<time_t>(); + long nsec = provider.ConsumeIntegral<long>(); + timespec timestamp = {.tv_sec = sec, .tv_nsec = nsec}; + android::status_t transactionStatus = provider.ConsumeIntegral<android::status_t>(); + + std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>( + provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); + + // same options so that FDs and binders could be shared in both Parcels + android::RandomParcelOptions options; + + android::Parcel p0, p1; + fillRandomParcel(&p0, FuzzedDataProvider(bytes.data(), bytes.size()), &options); + fillRandomParcel(&p1, std::move(provider), &options); + + auto transaction = + android::binder::debug::RecordedTransaction::fromDetails(interfaceName, code, flags, + timestamp, p0, p1, + transactionStatus); + + if (transaction.has_value()) { + std::FILE* intermediateFile = std::tmpfile(); + android::base::unique_fd fdForWriting(fileno(intermediateFile)); + auto writeStatus ATTRIBUTE_UNUSED = transaction.value().dumpToFile(fdForWriting); + + std::fclose(intermediateFile); + } + + return 0; +} diff --git a/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/power_recording b/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/power_recording Binary files differnew file mode 100644 index 0000000000..79442078c2 --- /dev/null +++ b/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/power_recording diff --git a/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/recorded_binder_transaction b/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/recorded_binder_transaction Binary files differnew file mode 100644 index 0000000000..658addbed5 --- /dev/null +++ b/libs/binder/tests/unit_fuzzers/recorded_transaction_corpus/recorded_binder_transaction diff --git a/libs/fakeservicemanager/Android.bp b/libs/fakeservicemanager/Android.bp index 29924ff500..96dcce11ea 100644 --- a/libs/fakeservicemanager/Android.bp +++ b/libs/fakeservicemanager/Android.bp @@ -11,7 +11,7 @@ cc_defaults { name: "fakeservicemanager_defaults", host_supported: true, srcs: [ - "ServiceManager.cpp", + "FakeServiceManager.cpp", ], shared_libs: [ @@ -28,7 +28,7 @@ cc_defaults { cc_library { name: "libfakeservicemanager", defaults: ["fakeservicemanager_defaults"], - export_include_dirs: ["include/fakeservicemanager"], + export_include_dirs: ["include"], } cc_test_host { @@ -38,5 +38,5 @@ cc_test_host { "test_sm.cpp", ], static_libs: ["libgmock"], - local_include_dirs: ["include/fakeservicemanager"], + local_include_dirs: ["include"], } diff --git a/libs/fakeservicemanager/ServiceManager.cpp b/libs/fakeservicemanager/FakeServiceManager.cpp index 1109ad8594..3272bbc1aa 100644 --- a/libs/fakeservicemanager/ServiceManager.cpp +++ b/libs/fakeservicemanager/FakeServiceManager.cpp @@ -14,18 +14,18 @@ * limitations under the License. */ -#include "ServiceManager.h" +#include "fakeservicemanager/FakeServiceManager.h" namespace android { -ServiceManager::ServiceManager() {} +FakeServiceManager::FakeServiceManager() {} -sp<IBinder> ServiceManager::getService( const String16& name) const { +sp<IBinder> FakeServiceManager::getService( const String16& name) const { // Servicemanager is single-threaded and cannot block. This method exists for legacy reasons. return checkService(name); } -sp<IBinder> ServiceManager::checkService( const String16& name) const { +sp<IBinder> FakeServiceManager::checkService( const String16& name) const { auto it = mNameToService.find(name); if (it == mNameToService.end()) { return nullptr; @@ -33,7 +33,7 @@ sp<IBinder> ServiceManager::checkService( const String16& name) const { return it->second; } -status_t ServiceManager::addService(const String16& name, const sp<IBinder>& service, +status_t FakeServiceManager::addService(const String16& name, const sp<IBinder>& service, bool /*allowIsolated*/, int /*dumpsysFlags*/) { if (service == nullptr) { @@ -43,7 +43,7 @@ status_t ServiceManager::addService(const String16& name, const sp<IBinder>& ser return NO_ERROR; } -Vector<String16> ServiceManager::listServices(int /*dumpsysFlags*/) { +Vector<String16> FakeServiceManager::listServices(int /*dumpsysFlags*/) { Vector<String16> services; for (auto const& [name, service] : mNameToService) { (void) service; @@ -52,19 +52,19 @@ Vector<String16> ServiceManager::listServices(int /*dumpsysFlags*/) { return services; } -IBinder* ServiceManager::onAsBinder() { +IBinder* FakeServiceManager::onAsBinder() { return nullptr; } -sp<IBinder> ServiceManager::waitForService(const String16& name) { +sp<IBinder> FakeServiceManager::waitForService(const String16& name) { return checkService(name); } -bool ServiceManager::isDeclared(const String16& name) { +bool FakeServiceManager::isDeclared(const String16& name) { return mNameToService.find(name) != mNameToService.end(); } -Vector<String16> ServiceManager::getDeclaredInstances(const String16& name) { +Vector<String16> FakeServiceManager::getDeclaredInstances(const String16& name) { Vector<String16> out; const String16 prefix = name + String16("/"); for (const auto& [registeredName, service] : mNameToService) { @@ -76,38 +76,38 @@ Vector<String16> ServiceManager::getDeclaredInstances(const String16& name) { return out; } -std::optional<String16> ServiceManager::updatableViaApex(const String16& name) { +std::optional<String16> FakeServiceManager::updatableViaApex(const String16& name) { (void)name; return std::nullopt; } -Vector<String16> ServiceManager::getUpdatableNames(const String16& apexName) { +Vector<String16> FakeServiceManager::getUpdatableNames(const String16& apexName) { (void)apexName; return {}; } -std::optional<IServiceManager::ConnectionInfo> ServiceManager::getConnectionInfo( +std::optional<IServiceManager::ConnectionInfo> FakeServiceManager::getConnectionInfo( const String16& name) { (void)name; return std::nullopt; } -status_t ServiceManager::registerForNotifications(const String16&, +status_t FakeServiceManager::registerForNotifications(const String16&, const sp<LocalRegistrationCallback>&) { return INVALID_OPERATION; } -status_t ServiceManager::unregisterForNotifications(const String16&, +status_t FakeServiceManager::unregisterForNotifications(const String16&, const sp<LocalRegistrationCallback>&) { return INVALID_OPERATION; } -std::vector<IServiceManager::ServiceDebugInfo> ServiceManager::getServiceDebugInfo() { +std::vector<IServiceManager::ServiceDebugInfo> FakeServiceManager::getServiceDebugInfo() { std::vector<IServiceManager::ServiceDebugInfo> ret; return ret; } -void ServiceManager::clear() { +void FakeServiceManager::clear() { mNameToService.clear(); } } // namespace android diff --git a/libs/fakeservicemanager/include/fakeservicemanager/ServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h index ba6bb7d95b..97add24ac8 100644 --- a/libs/fakeservicemanager/include/fakeservicemanager/ServiceManager.h +++ b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h @@ -28,9 +28,9 @@ namespace android { * A local host simple implementation of IServiceManager, that does not * communicate over binder. */ -class ServiceManager : public IServiceManager { +class FakeServiceManager : public IServiceManager { public: - ServiceManager(); + FakeServiceManager(); sp<IBinder> getService( const String16& name) const override; diff --git a/libs/fakeservicemanager/test_sm.cpp b/libs/fakeservicemanager/test_sm.cpp index 8682c1c795..6fc21c65d1 100644 --- a/libs/fakeservicemanager/test_sm.cpp +++ b/libs/fakeservicemanager/test_sm.cpp @@ -21,14 +21,14 @@ #include <binder/ProcessState.h> #include <binder/IServiceManager.h> -#include "ServiceManager.h" +#include "fakeservicemanager/FakeServiceManager.h" using android::sp; using android::BBinder; using android::IBinder; using android::OK; using android::status_t; -using android::ServiceManager; +using android::FakeServiceManager; using android::String16; using android::IServiceManager; using testing::ElementsAre; @@ -45,19 +45,19 @@ static sp<IBinder> getBinder() { } TEST(AddService, HappyHappy) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->addService(String16("foo"), getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT), OK); } TEST(AddService, SadNullBinder) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->addService(String16("foo"), nullptr, false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT), android::UNEXPECTED_NULL); } TEST(AddService, HappyOverExistingService) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->addService(String16("foo"), getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT), OK); EXPECT_EQ(sm->addService(String16("foo"), getBinder(), false /*allowIsolated*/, @@ -65,7 +65,7 @@ TEST(AddService, HappyOverExistingService) { } TEST(AddService, HappyClearAddedService) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->addService(String16("foo"), getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT), OK); EXPECT_NE(sm->getService(String16("foo")), nullptr); @@ -74,7 +74,7 @@ TEST(AddService, HappyClearAddedService) { } TEST(GetService, HappyHappy) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); sp<IBinder> service = getBinder(); EXPECT_EQ(sm->addService(String16("foo"), service, false /*allowIsolated*/, @@ -84,13 +84,13 @@ TEST(GetService, HappyHappy) { } TEST(GetService, NonExistant) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->getService(String16("foo")), nullptr); } TEST(ListServices, AllServices) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->addService(String16("sd"), getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT), OK); @@ -109,13 +109,13 @@ TEST(ListServices, AllServices) { } TEST(WaitForService, NonExistant) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_EQ(sm->waitForService(String16("foo")), nullptr); } TEST(WaitForService, HappyHappy) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); sp<IBinder> service = getBinder(); EXPECT_EQ(sm->addService(String16("foo"), service, false /*allowIsolated*/, @@ -125,13 +125,13 @@ TEST(WaitForService, HappyHappy) { } TEST(IsDeclared, NonExistant) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); EXPECT_FALSE(sm->isDeclared(String16("foo"))); } TEST(IsDeclared, HappyHappy) { - auto sm = new ServiceManager(); + auto sm = new FakeServiceManager(); sp<IBinder> service = getBinder(); EXPECT_EQ(sm->addService(String16("foo"), service, false /*allowIsolated*/, diff --git a/libs/graphicsenv/OWNERS b/libs/graphicsenv/OWNERS index 347c4e0db1..1db8cbe52f 100644 --- a/libs/graphicsenv/OWNERS +++ b/libs/graphicsenv/OWNERS @@ -1,10 +1,4 @@ -abdolrashidi@google.com -cclao@google.com chrisforbes@google.com cnorthrop@google.com ianelliott@google.com -lfy@google.com lpy@google.com -romanl@google.com -vantablack@google.com -yuxinhu@google.com diff --git a/libs/ui/Gralloc4.cpp b/libs/ui/Gralloc4.cpp index f6ab7b2a5e..53372c9866 100644 --- a/libs/ui/Gralloc4.cpp +++ b/libs/ui/Gralloc4.cpp @@ -22,6 +22,8 @@ #include <aidlcommonsupport/NativeHandle.h> #include <android/binder_enums.h> #include <android/binder_manager.h> +#include <cutils/android_filesystem_config.h> +#include <cutils/multiuser.h> #include <gralloctypes/Gralloc4.h> #include <hidl/ServiceManagement.h> #include <hwbinder/IPCThreadState.h> @@ -1195,8 +1197,15 @@ Gralloc4Allocator::Gralloc4Allocator(const Gralloc4Mapper& mapper) : mMapper(map mAllocator = IAllocator::getService(); if (__builtin_available(android 31, *)) { if (hasIAllocatorAidl()) { - mAidlAllocator = AidlIAllocator::fromBinder(ndk::SpAIBinder( - AServiceManager_waitForService(kAidlAllocatorServiceName.c_str()))); + // TODO(b/269517338): Perform the isolated checking for this in service manager instead. + uid_t aid = multiuser_get_app_id(getuid()); + if (aid >= AID_ISOLATED_START && aid <= AID_ISOLATED_END) { + mAidlAllocator = AidlIAllocator::fromBinder(ndk::SpAIBinder( + AServiceManager_getService(kAidlAllocatorServiceName.c_str()))); + } else { + mAidlAllocator = AidlIAllocator::fromBinder(ndk::SpAIBinder( + AServiceManager_waitForService(kAidlAllocatorServiceName.c_str()))); + } ALOGE_IF(!mAidlAllocator, "AIDL IAllocator declared but failed to get service"); } } diff --git a/opengl/OWNERS b/opengl/OWNERS index 379f7638f0..3d60a1dad6 100644 --- a/opengl/OWNERS +++ b/opengl/OWNERS @@ -1,11 +1,6 @@ -abdolrashidi@google.com -cclao@google.com chrisforbes@google.com cnorthrop@google.com ianelliott@google.com jessehall@google.com -lfy@google.com lpy@google.com -romanl@google.com vantablack@google.com -yuxinhu@google.com diff --git a/opengl/libs/EGL/BlobCache.cpp b/opengl/libs/EGL/BlobCache.cpp index 86c788d3b3..aecfc6b077 100644 --- a/opengl/libs/EGL/BlobCache.cpp +++ b/opengl/libs/EGL/BlobCache.cpp @@ -231,7 +231,7 @@ int BlobCache::flatten(void* buffer, size_t size) const { int BlobCache::unflatten(void const* buffer, size_t size) { // All errors should result in the BlobCache being in an empty state. - mCacheEntries.clear(); + clear(); // Read the cache header if (size < sizeof(Header)) { @@ -258,7 +258,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) { size_t numEntries = header->mNumEntries; for (size_t i = 0; i < numEntries; i++) { if (byteOffset + sizeof(EntryHeader) > size) { - mCacheEntries.clear(); + clear(); ALOGE("unflatten: not enough room for cache entry headers"); return -EINVAL; } @@ -270,7 +270,7 @@ int BlobCache::unflatten(void const* buffer, size_t size) { size_t totalSize = align4(entrySize); if (byteOffset + totalSize > size) { - mCacheEntries.clear(); + clear(); ALOGE("unflatten: not enough room for cache entry headers"); return -EINVAL; } diff --git a/opengl/libs/EGL/BlobCache.h b/opengl/libs/EGL/BlobCache.h index ff03d30099..52078ff5fd 100644 --- a/opengl/libs/EGL/BlobCache.h +++ b/opengl/libs/EGL/BlobCache.h @@ -117,7 +117,10 @@ public: // clear flushes out all contents of the cache then the BlobCache, leaving // it in an empty state. - void clear() { mCacheEntries.clear(); } + void clear() { + mCacheEntries.clear(); + mTotalSize = 0; + } protected: // mMaxTotalSize is the maximum size that all cache entries can occupy. This diff --git a/opengl/libs/EGL/BlobCache_test.cpp b/opengl/libs/EGL/BlobCache_test.cpp index ceea0fb979..450c12837b 100644 --- a/opengl/libs/EGL/BlobCache_test.cpp +++ b/opengl/libs/EGL/BlobCache_test.cpp @@ -466,4 +466,31 @@ TEST_F(BlobCacheFlattenTest, UnflattenCatchesBufferTooSmall) { ASSERT_EQ(size_t(0), mBC2->get("abcd", 4, buf, 4)); } +// Test for a divide by zero bug (b/239862516). Before the fix, unflatten() would not reset +// mTotalSize when it encountered an error, which would trigger division by 0 in clean() in the +// right conditions. +TEST_F(BlobCacheFlattenTest, SetAfterFailedUnflatten) { + // isCleanable() must be true, so mTotalSize must be > mMaxTotalSize / 2 after unflattening + // after one entry is lost. To make this the case, MaxTotalSize is 30 and three 10 sized + // entries are used. One of those entries is lost, resulting in mTotalSize=20 + const size_t kMaxKeySize = 10; + const size_t kMaxValueSize = 10; + const size_t kMaxTotalSize = 30; + mBC.reset(new BlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize)); + mBC2.reset(new BlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize)); + mBC->set("aaaaa", 5, "aaaaa", 5); + mBC->set("bbbbb", 5, "bbbbb", 5); + mBC->set("ccccc", 5, "ccccc", 5); + + size_t size = mBC->getFlattenedSize(); + uint8_t* flat = new uint8_t[size]; + ASSERT_EQ(OK, mBC->flatten(flat, size)); + + ASSERT_EQ(BAD_VALUE, mBC2->unflatten(flat, size - 10)); + delete[] flat; + + // This line will trigger clean() which caused a crash. + mBC2->set("dddddddddd", 10, "dddddddddd", 10); +} + } // namespace android diff --git a/services/batteryservice/include/batteryservice/BatteryService.h b/services/batteryservice/include/batteryservice/BatteryService.h index a2e4115bd6..bf6189d7af 100644 --- a/services/batteryservice/include/batteryservice/BatteryService.h +++ b/services/batteryservice/include/batteryservice/BatteryService.h @@ -37,6 +37,7 @@ enum { BATTERY_PROP_CHARGING_POLICY = 7, // equals BATTERY_PROPERTY_CHARGING_POLICY BATTERY_PROP_MANUFACTURING_DATE = 8, // equals BATTERY_PROPERTY_MANUFACTURING_DATE BATTERY_PROP_FIRST_USAGE_DATE = 9, // equals BATTERY_PROPERTY_FIRST_USAGE_DATE + BATTERY_PROP_STATE_OF_HEALTH = 10, // equals BATTERY_PROPERTY_STATE_OF_HEALTH }; struct BatteryProperties { diff --git a/services/sensorservice/aidl/fuzzer/fuzzer.cpp b/services/sensorservice/aidl/fuzzer/fuzzer.cpp index 1b63d76953..ee8ceb354c 100644 --- a/services/sensorservice/aidl/fuzzer/fuzzer.cpp +++ b/services/sensorservice/aidl/fuzzer/fuzzer.cpp @@ -16,7 +16,7 @@ #include <fuzzbinder/libbinder_ndk_driver.h> #include <fuzzer/FuzzedDataProvider.h> -#include <ServiceManager.h> +#include <fakeservicemanager/FakeServiceManager.h> #include <android-base/logging.h> #include <android/binder_interface_utils.h> #include <fuzzbinder/random_binder.h> @@ -29,7 +29,7 @@ using ndk::SharedRefBase; [[clang::no_destroy]] static std::once_flag gSmOnce; extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - static android::sp<android::ServiceManager> fakeServiceManager = new android::ServiceManager(); + static android::sp<android::FakeServiceManager> fakeServiceManager = new android::FakeServiceManager(); std::call_once(gSmOnce, [&] { setDefaultServiceManager(fakeServiceManager); }); fakeServiceManager->clear(); |