summaryrefslogtreecommitdiff
path: root/libs/binder/Parcel.cpp
Commit message (Collapse)AuthorAgeFilesLines
* libbinder: Parcel: validate read data before writeSteven Moreland2025-01-131-0/+12
| | | | | | | | | | | | | This is slow, but it's required to prevent memory corruption. Ignore-AOSP-First: security Bug: 370840874 Test: fuzzer (cherry picked from commit c54dad65317f851ce9d016bd90ec6a7a04da09fc) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:640d942d6a9a26a0beca87d3abdf2ee1048985b9) Merged-In: Ibc5566ade0389221690dc90324f93394cf7fc9a5 Change-Id: Ibc5566ade0389221690dc90324f93394cf7fc9a5
* libbinder: Parcel: grow rejects large data posSteven Moreland2025-01-131-0/+8
| | | | | | | | | | | | | This is unexpected behavior so throw an error. Allocating this much memory may cause OOM or other issues. Bug: 370831157 Test: fuzzer (cherry picked from commit 608524d462278c2c9f6716cd94f126c85e9f2e91) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:93856b3c6df5a34717dddddfab23d945753f7de8) Merged-In: Iea0884ca61b08e52e6a6e9c66693e427cb5536f4 Change-Id: Iea0884ca61b08e52e6a6e9c66693e427cb5536f4
* binder: fix FD handling in continueWriteFrederick Mayle2025-01-131-9/+57
| | | | | | | | | | | | | | | | | | | | | | | | | Only close FDs within the truncated part of the parcel. This change also fixes a bug where a parcel truncated into the middle of an object would not properly free that object. That could have resulted in an OOB access in `Parcel::truncateRpcObjects`, so more bounds checking is added. The new tests show how to reproduce the bug by appending to or partially truncating Parcels owned by the kernel. Two cases are disabled because of a bug in the Parcel fdsan code (b/370824489). Cherry-pick notes: Add truncateFileDescriptors method instead of modifying closeFileDescriptors to avoid API change errors. Tweaked the test to support older C++ and android-base libs. Flag: EXEMPT bugfix Ignore-AOSP-First: security fix Bug: 239222407, 359179312 Test: atest binderLibTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e77c9487166599cce197597038011f0879770ab4) Merged-In: Iadf7e2e98e3eb97c56ec2fed2b49d1e6492af9a3 Change-Id: Iadf7e2e98e3eb97c56ec2fed2b49d1e6492af9a3
* Merge "Minor build-outside-android fixes" into main am: 43571dd2ef am: ↵Tomasz Wasilczyk2023-12-071-1/+1
|\ | | | | | | | | | | | | | | | | dda7686ebb am: 8ff6f889dc Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2852823 Change-Id: I3ce78f71a50097e1e0e7f177a9e8a511c9b619a6 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Merge "Minor build-outside-android fixes" into mainTomasz Wasilczyk2023-12-061-1/+1
| |\
| | * Minor build-outside-android fixesTomasz Wasilczyk2023-12-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - add missing includes - fix maybe_unused warning - use LOG_ALWAYS_FATAL instead of __assert (behind flag) Bug: 302723053 Test: mma in binder folder Change-Id: I4e90ff7c7f37e6736bc38abaa11744ccf7155a17
* | | Merge "libbinder: restrict non-Android kernel binder use" into main am: ↵Steven Moreland2023-12-061-0/+11
|\| | | | | | | | | | | | | | | | | | | | | | | | | | 96b83024e4 am: 3969bc7e77 am: e712783ddc Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2860905 Change-Id: I6733149035dabe2e6d7fe47134df7a77fe8d97f9 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * | libbinder: restrict non-Android kernel binder useSteven Moreland2023-12-051-0/+11
| |/ | | | | | | | | | | | | | | | | | | Bringing this up in new environments, we need to make sure they use different headers, so we never copy transactions from one environment or another by accident. Bug: 313702213 Test: build Change-Id: I0cae4a149267862092030c00d239e93155f70143
* | Merge "Binder unique_fd" into main am: 1cc6ef1a50 am: b59b0e8323 am: d66061b7f9Treehugger Robot2023-11-211-18/+31
|\| | | | | | | | | | | | | Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2813342 Change-Id: I64b6511d1bb87247f418f5de5ede4d85be58f9cf Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Binder unique_fdTomasz Wasilczyk2023-11-171-18/+31
| | | | | | | | | | | | Test: mma Bug: 302723053 Change-Id: I52f14cadb027b3f854946d5315dce3d23aa21b19
* | Merge changes from topic ↵Tomasz Wasilczyk2023-11-051-2/+4
|\| | | | | | | | | | | | | | | | | "revert-2807644-revert-2780893-XRITMVSTFB-ZYEEKMIRIQ" into main am: dc0f937ddf am: d6ca48dcab am: 124fd63f89 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2809894 Change-Id: I4f9ae094682ebb7649dae05d04ad85ec565d2cda Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Revert^2 "Use std::unique_ptr instead of ScopeGuard"Tomasz Wasilczyk2023-11-041-2/+4
| | | | | | | | | | | | | | | | 25c1a3b8543dd1756308424dd65030f90bb7a99f Test: m Bug: 302723053 Change-Id: Id9355c10d78d0c55afb49f512b78bb0923fbc4f7
* | Merge "Binder: don't depend on libutils headers" into main am: d429f318b4 ↵Tomasz Wasilczyk2023-10-311-2/+1
|\| | | | | | | | | | | | | | | | | am: 2f1ac888ec am: f224612600 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2802651 Change-Id: Ib4effc268401fd8c09de27f88f2c80729eebb335 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Binder: don't depend on libutils headersTomasz Wasilczyk2023-10-311-2/+1
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: Id68a10a491e3db7f27ea2cbf843078544bb0ab85
* | Merge "Binder: Flattenable is no longer a hard dependency" into main am: ↵Tomasz Wasilczyk2023-10-311-1/+0
|\| | | | | | | | | | | | | | | | | f334e5da6d am: 7eaf64f8ed am: e6bf9ec7e7 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2807155 Change-Id: Icaabf54dfc20d432d7c1b23fc82824211941bf07 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Binder: Flattenable is no longer a hard dependencyTomasz Wasilczyk2023-10-311-1/+0
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: I0dddbd085b1999c3eb11577e6120ea422be4c10a
* | Merge changes from topic "revert-2780893-XRITMVSTFB" into main am: ↵Sebastian Pickl2023-10-301-4/+2
|\| | | | | | | | | | | | | | | | | 271bd23714 am: 4ad3f2b2ef am: e75d125951 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2807644 Change-Id: If83584db96e59d57965a65f20b257e7c4839dde1 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Revert "Use std::unique_ptr instead of ScopeGuard"Sebastian Pickl2023-10-301-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | Revert submission 2780893 Reason for revert: breaking boot tests Bug: 308214260 Reverted changes: /q/submissionid:2780893 Change-Id: I7a4ee9a45583a8a1d4a33447de55c63e6ce9d42a
* | Merge changes I2a437363,I27226885 into main am: 9636af27d2 am: 706b154396 ↵Treehugger Robot2023-10-281-2/+4
|\| | | | | | | | | | | | | | | | | am: 01f261380d Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2780893 Change-Id: Ic227eca26dd7a4d3cf45e8c864f43a8a248e8844 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Use std::unique_ptr instead of ScopeGuardTomasz Wasilczyk2023-10-271-2/+4
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: I27226885b8b5e771d675ba2d83d0a2e14551d13e
* | Merge "Disable native_handle outside of Android" into main am: 5c05de1968 ↵Tomasz Wasilczyk2023-10-201-0/+4
|\| | | | | | | | | | | | | | | | | am: 50af66c545 am: d924869ae6 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2779429 Change-Id: I1212495972fd37e040cbb85747f45cd47cba255d Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Disable native_handle outside of AndroidTomasz Wasilczyk2023-10-191-0/+4
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: Ia6d80574a3b137c7646b4a8a7575e03197fcb527
* | Merge "Disable Blob outside of Android" into main am: 5bd4fecba9 am: ↵Tomasz Wasilczyk2023-10-191-0/+17
|\| | | | | | | | | | | | | | | | | e96bb74514 am: 7f68d9da8f Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2779430 Change-Id: I4ebb5f72ec3a2db03aea23fd18e59f94f5ee7b2a Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Disable Blob outside of AndroidTomasz Wasilczyk2023-10-191-0/+17
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: I2abc138831da28783b26efe75ee6f4583e263692
* | Merge "Binder: Split OS to Android and Unix part, abstract GetThreadId out" ↵Tomasz Wasilczyk2023-10-171-5/+5
|\| | | | | | | | | | | | | | | | | into main am: 3f4c4957a6 am: 4792f034f3 am: 15d7bc7d43 am: 27138eb395 am: 8f7cc14d21 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2776819 Change-Id: I912047d8711101d2a8c9d4f690622c06bb4b865a Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Binder: Split OS to Android and Unix part, abstract GetThreadId outTomasz Wasilczyk2023-10-161-5/+5
| | | | | | | | | | | | | | Bug: 302723053 Test: mma in binder folder Test: aosp/2616196 Change-Id: I73df8fc453df0edf496960853cb0004f1c3a6a43
* | Merge "Use C++20 [[unlikely]] instead of UNLIKELY macro" into main am: ↵Tomasz Wasilczyk2023-10-111-1/+0
|\| | | | | | | | | | | | | | | | | 56f83aa426 am: e963d6b2d6 am: 3829544d2c am: 2362ad9738 am: 28b7a7e346 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2779433 Change-Id: Ic2312fd34ef66ff4356eb617d1dfe798f09dbf17 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Use C++20 [[unlikely]] instead of UNLIKELY macroTomasz Wasilczyk2023-10-091-1/+0
| | | | | | | | | | | | Bug: 302723053 Test: mma Change-Id: I126772474763a55f43de0cd4a3d2605f7f84da3b
* | Merge "Use String8/16 c_str [binder]" into main am: a79915269d am: ↵Treehugger Robot2023-08-251-6/+6
|\| | | | | | | | | | | | | | | | | 13da0f9a0f am: 7e5716d5fd am: 5f539c068f am: 132e927860 Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2704458 Change-Id: I25e93d8f301178b4a45e6a5b82f2ff4abf99ff9a Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
| * Use String8/16 c_str [binder]Tomasz Wasilczyk2023-08-251-6/+6
| | | | | | | | | | | | | | | | | | | | Current String8::string() has two problems: it may suggest it's returning a std::string and also prevents a drop-in replacement with std::string. Bug: 295394788 Test: make checkbuild Change-Id: I1eb6ddebe3faede57f3e6f046da572a79056125a
* | Merge "Revert^2 "Parcel: fdsan""Steven Moreland2023-06-301-3/+49
|\ \ | |/ |/|
| * Revert^2 "Parcel: fdsan"Steven Moreland2023-06-221-3/+49
| | | | | | | | | | | | 734537727237ec6217241520f66229630359d303 Change-Id: I8e3df46d474c857a76fb00ab195d73dbea282520
| * Revert "Parcel: fdsan"Priyanka Advani2023-06-221-49/+3
| | | | | | | | | | | | | | | | | | | | Revert submission 23699976-fdsan-parcel Reason for revert: Possible culprit for b/288448299 Reverted changes: /q/submissionid:23699976-fdsan-parcel Change-Id: I8d7b4e222598436271ee03cfc8826910327c98aa
| * Parcel: fdsanSteven Moreland2023-06-151-3/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Marks FDs as owned by the Parcel for clearer errors if other things accidentally own them. Bug: 287093457 Test: boot Test: binderUnitTest Test: binder_parcel_fuzzer (several minutes on-device) Ignore-AOSP-First: this requires some fixes only in git_master to avoid crashing Change-Id: Ibbcc7fdf074a0a1a61ae11f61c8b11f8445aef20
* | libbinder_ndk: fwd fuzzing status to NDK bindersSteven Moreland2023-06-281-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When passing binders into NDK backend services, we always type check them immediately. This allows errors to show up earlier there, but may be inefficient because the type will also be checked on every transaction. Anyway... This poses a problem for our automatic fuzzers because callbacks passed into services (e.g. RandomBinder) will be ignored for NDK backend fuzzers unless they correctly guess their interface descriptor. There are a few things we could do: - use random strings from the environment - export a list of possible interface descriptors from AIDL - generate our corpuses from other data However, the simplest thing we can do is ignore the check, which this CL does. Of course, it isn't great to continue differentiated fuzzer behavior from actual behavior, so we'd like to revert this once we have a more comprehensive solution. However, callbacks are a fundamental AIDL building blocks, so forcing good fuzzer coverage for these pieces seems justified. Bug: N/A Test: I added an abort in an NDK backend service. Without this change, that path is never found, but with this change, it was hit after only ~6,000 iterations. Change-Id: I4cbe5c56b93b9300fbd57e72e24075c02df38ba9
* | service fuzzer: ignore interface checksSteven Moreland2023-06-021-2/+8
| | | | | | | | | | | | | | | | | | | | | | We bypass this half the time in the driver now, but it's better to bypass it here in Parcel so that we fuzz how different fields are set at the beginning of Parcel. Bug: N/A Test: servicemanager_fuzzer for several minutes Change-Id: I17399680c0f5d9239071e9d4fa6bcedb545f7871
* | Parcel: service fuzzing modeSteven Moreland2023-06-021-13/+40
|/ | | | | | | | | | | | | | In service fuzzing mode, we disable logs in the core Parcel code and also limit some API checks. This increases fuzzer efficiency (~2x or more, ignoring service-specific inefficiencies) just due to bypassing the 'mixing copies of libbinder' check, which isn't critical for fuzzing. Bug: N/A Test: manual, diffing strace outputs Change-Id: I4952b3ab5f55f27f33d5124e39541fdd40143971
* Check for data buffer size while marshalling parcelPawan Wagh2023-01-231-0/+4
| | | | | | | | | | | | | Checking for internal buffer size which should handle cases where parcel has position set beyond datasize and data size is actually returning the data position. Test: m && acloud delete --all && acloud create --local-image --local-instance && atest -c CtsNdkBinderTestCases Test: m binder_parcel_fuzzer && out/host/linux-x86/fuzz/x86_64/binder_parcel_fuzzer/binder_parcel_fuzzer Bug: 264739302 Change-Id: Ib6c49fde1c1a56bae3932ce9af731a200b8a8faa
* Do not set FLAT_BINDER_FLAG_* on object of type BINDER_TYPE_FDT.J. Mercier2022-12-191-1/+1
| | | | | | | | | | | | | | | | | | Parcel::writeFileDescriptor should be creating a binder_fd_object and serializing it for submission to the binder driver. However the common Parcel::writeObject function takes a flat_binder_object as an argument, so writeFileDescriptor creates a flat_binder_object and uses the memory locations equivalent to the correct fields in binder_fd_object. That works, but in the process the flags field of the object is populated with FLAT_BINDER_FLAG_* values, which is incorrect for an object type of BINDER_TYPE_FD. The binder driver does not attempt to check that the flags field is populated only with valid flag bits (and this would be pretty difficult, since valid flag values overlap between the two types), but it also does not do anything with these invalid flags. So here we remove them. Test: Cuttlefish boots successfuly Change-Id: I1ba07855976097f7b99ea78526c933df42be68ba
* Merge "libbinder: Additional dupFileDescriptor use in Parcel.cpp"Treehugger Robot2022-11-301-3/+6
|\
| * libbinder: Additional dupFileDescriptor use in Parcel.cppAndrei Homescu2022-11-291-3/+6
| | | | | | | | | | | | | | | | | | Use the new dupFileDescriptor API in one additional place in Parcel.cpp to make the code more portable. Bug: 230135749 Test: presubmit Change-Id: I41c18c1e319eb7ad06c90d6a495bad31bd677a6d
* | Skipping enforceNoDataAvail in fuzzServicePawan Wagh2022-11-171-0/+9
|/ | | | | | | | | | | | Adding API to skip dataAvail check and using it in fuzzService. Bug: 241848255 Test: m binderUnitTest && out/host/linux-x86/nativetest64/binderUnitTest/binderUnitTest Test: m servicemanager_fuzzer && out/host/linux-x86/fuzz/x86_64/servicemanager_fuzzer/servicemanager_fuzzer Change-Id: Ib43936ff4a7dca4a036d34b3e475d553f3d21be2
* libbinder: add ancillaryFd support in RpcTransportTipcTrustyAndrei Homescu2022-10-051-1/+2
| | | | | | | | | Implement ParcelFileDescriptor support in Trusty by adding support for ancillaryFd to RpcTransportTipcTrusty. Bug: 242940548 Test: Trusty tests Change-Id: Ic5602bbf9f239f65489e0f411f89b10907741be3
* libbinder : Use logging from liblogPawan Wagh2022-09-281-5/+3
| | | | | | | | | | TextOutput based logs are causing direct leaks while fuzzing service manager. Modifying logs to use liblog based logging functions. BUG: 240388906 Test: m SANITIZE_HOST=address servicemanager_fuzzer && out/host/linux-x86/fuzz/x86_64/servicemanager_fuzzer/servicemanager_fuzzer Change-Id: I3721051bc92df86c80dce9ed2e9dcac20cafef84
* libbinder: build ParcelFileDescriptor on TrustyAndrei Homescu2022-08-161-8/+19
| | | | | | | | | | | | | | Adds ParcelFileDescriptor.cpp to the Trusty build which in turn compiles some extra related code from Parcel.cpp. This code uses fcntl(...O_DUPFD_CLOEXEC...) to duplicate file descriptors which is not available on Trusty (or other OSes), so we abstract the relevant calls to that function in a wrapper in OS.cpp. Bug: 224644083 Test: m Test: build on Trusty Change-Id: I2e0ee517e7c7b55458a6ee6db977985aab4d7c58
* libbinder: Validate the RPC object positions immediatelyFrederick Mayle2022-07-151-6/+16
| | | | | | | | | | We already check the bounds when they are used, but it is easier to reason about the code if they are validated before being stored. The `freeData` call was redundant (handled by dtor). Test: TH Change-Id: I5160e72b5225b933068a3aa68eb5287bf7a29af1
* libbinder: Remove Parcel argument from Parcel::release_funcFrederick Mayle2022-07-151-3/+10
| | | | | | | | | | This enforces a clearer separation of concerns between the data owner and the parcel code. It happened to reveal an edge case where FDs are prematurely closed (tracking a fix in b/239222407). Test: TH Bug: 239222407 Change-Id: I54ff0c8e4a8f64afd529092d2038dac40c853371
* libbinder: abort on Parcel::appendFrom in no-kernel modeAndrei Homescu2022-07-081-2/+5
| | | | | | | | | Abort with LOG_ALWAYS_FATAL if Parcel::appendFrom is called on a kernel Parcel in a no-kernel build. Bug: 224644083 Test: build on Trusty Change-Id: I8a63b23fbd465de8d0e89597622692f7266ed0bf
* libbinder: build option to disable the kernel IPC, part 2/2Andrei Homescu2022-07-011-4/+107
| | | | | | | | | Use BINDER_KERNEL_IPC to disable all code that uses flat_binder_object in Parcel.cpp. Bug: 224644083 Test: m Change-Id: I1f676dbfb464fc0c5eaa6b5266b7af1dabe984e1
* libbinder: Stricter protocol and code for receiving FDsFrederick Mayle2022-06-301-9/+6
| | | | | | | | | | | | | | | | | This is a slight change the to wire protocol. Now out-of-band FDs must be sent along with the command header bytes. The code changes exploit that by only using the more complex `recvmsg` call when reading the command header. Additionally, we explicitly pass around the list of FDs so that there is no risk of accumulating them. The same (somewhat ugly) vector type is used everywhere now so that there is only one allocation to capture the FDs and pass them to the `Parcel` object. Test: binderRpcTest Bug: 185909244 Change-Id: I1f55995ca82338ab9716fb2246c954ac8b16cfe5
generated by cgit v1.2.3 (git 2.25.1) at 2026-01-12 05:29:09 +0000