Skip to content

Commit b09a1d2

Browse files
author
Steven Moreland
committed
binder_parcel_fuzzer: support owned Parcels
Some codepaths in Parcel are different when the Parcel is owned. This allows fillRandomParcel to also sometimes cause the returned Parcel be a view of a filled out Parcel. This increases the possible impact and bug finding abilities of all of our AIDL fuzzers as well. Ignore-AOSP-First: fuzzing Bug: 369404061 Test: binder_parcel_fuzzer Change-Id: Ib19a0cbd74d48e18ba36cff56202541105ef9163
1 parent 750d69b commit b09a1d2

6 files changed

Lines changed: 103 additions & 9 deletions

File tree

libs/binder/Parcel.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2728,6 +2728,65 @@ size_t Parcel::ipcObjectsCount() const
27282728
return 0;
27292729
}
27302730

2731+
static void do_nothing_release_func(const uint8_t* data, size_t dataSize,
2732+
const binder_size_t* objects, size_t objectsCount) {
2733+
(void)data;
2734+
(void)dataSize;
2735+
(void)objects;
2736+
(void)objectsCount;
2737+
}
2738+
static void delete_data_release_func(const uint8_t* data, size_t dataSize,
2739+
const binder_size_t* objects, size_t objectsCount) {
2740+
delete[] data;
2741+
(void)dataSize;
2742+
(void)objects;
2743+
(void)objectsCount;
2744+
}
2745+
2746+
void Parcel::makeDangerousViewOf(Parcel* p) {
2747+
if (p->isForRpc()) {
2748+
// warning: this must match the logic in rpcSetDataReference
2749+
auto* rf = p->maybeRpcFields();
2750+
LOG_ALWAYS_FATAL_IF(rf == nullptr);
2751+
std::vector<std::variant<binder::unique_fd, binder::borrowed_fd>> fds;
2752+
if (rf->mFds) {
2753+
fds.reserve(rf->mFds->size());
2754+
for (const auto& fd : *rf->mFds) {
2755+
fds.push_back(binder::borrowed_fd(toRawFd(fd)));
2756+
}
2757+
}
2758+
status_t result =
2759+
rpcSetDataReference(rf->mSession, p->mData, p->mDataSize,
2760+
rf->mObjectPositions.data(), rf->mObjectPositions.size(),
2761+
std::move(fds), do_nothing_release_func);
2762+
LOG_ALWAYS_FATAL_IF(result != OK, "Failed: %s", statusToString(result).c_str());
2763+
} else {
2764+
#ifdef BINDER_WITH_KERNEL_IPC
2765+
// warning: this must match the logic in ipcSetDataReference
2766+
auto* kf = p->maybeKernelFields();
2767+
LOG_ALWAYS_FATAL_IF(kf == nullptr);
2768+
2769+
// Ownership of FDs is passed to the Parcel from kernel binder. This should be refactored
2770+
// to move this ownership out of Parcel and into release_func. However, today, Parcel
2771+
// always assums it can own and close FDs today. So, for purposes of testing consistency,
2772+
// , create new FDs it can own.
2773+
2774+
uint8_t* newData = new uint8_t[p->mDataSize]; // deleted by delete_data_release_func
2775+
memcpy(newData, p->mData, p->mDataSize);
2776+
for (size_t i = 0; i < kf->mObjectsSize; i++) {
2777+
flat_binder_object* flat =
2778+
reinterpret_cast<flat_binder_object*>(newData + kf->mObjects[i]);
2779+
if (flat->hdr.type == BINDER_TYPE_FD) {
2780+
flat->handle = fcntl(flat->handle, F_DUPFD_CLOEXEC, 0);
2781+
}
2782+
}
2783+
2784+
ipcSetDataReference(newData, p->mDataSize, kf->mObjects, kf->mObjectsSize,
2785+
delete_data_release_func);
2786+
#endif // BINDER_WITH_KERNEL_IPC
2787+
}
2788+
}
2789+
27312790
void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
27322791
size_t objectsCount, release_func relFunc) {
27332792
// this code uses 'mOwner == nullptr' to understand whether it owns memory
@@ -2738,6 +2797,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, const bin
27382797
auto* kernelFields = maybeKernelFields();
27392798
LOG_ALWAYS_FATAL_IF(kernelFields == nullptr); // guaranteed by freeData.
27402799

2800+
// must match makeDangerousViewOf
27412801
mData = const_cast<uint8_t*>(data);
27422802
mDataSize = mDataCapacity = dataSize;
27432803
kernelFields->mObjects = const_cast<binder_size_t*>(objects);
@@ -2816,6 +2876,7 @@ status_t Parcel::rpcSetDataReference(
28162876
auto* rpcFields = maybeRpcFields();
28172877
LOG_ALWAYS_FATAL_IF(rpcFields == nullptr); // guaranteed by markForRpc.
28182878

2879+
// must match makeDangerousViewOf
28192880
mData = const_cast<uint8_t*>(data);
28202881
mDataSize = mDataCapacity = dataSize;
28212882
mOwner = relFunc;

libs/binder/include/binder/Parcel.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,11 @@ class Parcel {
649649

650650
LIBBINDER_EXPORTED void print(std::ostream& to, uint32_t flags = 0) const;
651651

652+
// This API is to quickly become a view of another Parcel, so that we can also
653+
// test 'owner' paths quickly. It's extremely dangerous to use this API in
654+
// practice, and you should never ever do it.
655+
LIBBINDER_EXPORTED void makeDangerousViewOf(Parcel* p);
656+
652657
private:
653658
// Close all file descriptors in the parcel at object positions >= newObjectsSize.
654659
void closeFileDescriptors(size_t newObjectsSize);
@@ -664,7 +669,7 @@ class Parcel {
664669
void ipcSetDataReference(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
665670
size_t objectsCount, release_func relFunc);
666671
// Takes ownership even when an error is returned.
667-
status_t rpcSetDataReference(
672+
[[nodiscard]] status_t rpcSetDataReference(
668673
const sp<RpcSession>& session, const uint8_t* data, size_t dataSize,
669674
const uint32_t* objectTable, size_t objectTableSize,
670675
std::vector<std::variant<binder::unique_fd, binder::borrowed_fd>>&& ancillaryFds,

libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_parcel.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ struct RandomParcelOptions {
2828
std::function<void(Parcel* p, FuzzedDataProvider& provider)> writeHeader;
2929
std::vector<sp<IBinder>> extraBinders;
3030
std::vector<binder::unique_fd> extraFds;
31+
32+
// internal state owned by fillRandomParcel, for Parcel views
33+
std::vector<std::unique_ptr<Parcel>> extraParcels;
3134
};
3235

3336
/**

libs/binder/tests/parcel_fuzzer/main.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ void doTransactFuzz(const char* backend, const sp<B>& binder, FuzzedDataProvider
7070
uint32_t code = provider.ConsumeIntegral<uint32_t>();
7171
uint32_t flag = provider.ConsumeIntegral<uint32_t>();
7272

73-
FUZZ_LOG() << "backend: " << backend;
73+
FUZZ_LOG() << "doTransactFuzz backend: " << backend;
7474

7575
RandomParcelOptions options;
7676

@@ -101,7 +101,7 @@ void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads,
101101
// since we are only using a byte to index
102102
CHECK_LE(reads.size(), 255u) << reads.size();
103103

104-
FUZZ_LOG() << "backend: " << backend;
104+
FUZZ_LOG() << "doReadFuzz backend: " << backend;
105105
FUZZ_LOG() << "input: " << HexString(p.data(), p.dataSize());
106106
FUZZ_LOG() << "instructions: " << HexString(instructions.data(), instructions.size());
107107

@@ -122,10 +122,15 @@ void doReadWriteFuzz(const char* backend, const std::vector<ParcelRead<P>>& read
122122
RandomParcelOptions options;
123123
P p;
124124

125+
// small amount of initial Parcel data, since fillRandomParcel uses makeDangerousViewOf
126+
std::vector<uint8_t> parcelData =
127+
provider.ConsumeBytes<uint8_t>(provider.ConsumeIntegralInRange<size_t>(0, 20));
128+
fillRandomParcel(&p, FuzzedDataProvider(parcelData.data(), parcelData.size()), &options);
129+
125130
// since we are only using a byte to index
126131
CHECK_LE(reads.size() + writes.size(), 255u) << reads.size();
127132

128-
FUZZ_LOG() << "backend: " << backend;
133+
FUZZ_LOG() << "doReadWriteFuzz backend: " << backend;
129134

130135
while (provider.remaining_bytes() > 0) {
131136
uint8_t idx = provider.ConsumeIntegralInRange<uint8_t>(0, reads.size() + writes.size() - 1);

libs/binder/tests/parcel_fuzzer/random_parcel.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <fuzzbinder/random_parcel.h>
1818

1919
#include <android-base/logging.h>
20+
#include <binder/Functional.h>
2021
#include <binder/RpcSession.h>
2122
#include <binder/RpcTransportRaw.h>
2223
#include <fuzzbinder/random_binder.h>
@@ -32,10 +33,29 @@ static void fillRandomParcelData(Parcel* p, FuzzedDataProvider&& provider) {
3233
CHECK(OK == p->write(data.data(), data.size()));
3334
}
3435

35-
void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider, RandomParcelOptions* options) {
36+
void fillRandomParcel(Parcel* outputParcel, FuzzedDataProvider&& provider,
37+
RandomParcelOptions* options) {
3638
CHECK_NE(options, nullptr);
3739

38-
if (provider.ConsumeBool()) {
40+
const uint8_t fuzzerParcelOptions = provider.ConsumeIntegral<uint8_t>();
41+
const bool resultShouldBeView = fuzzerParcelOptions & 1;
42+
const bool resultShouldBeRpc = fuzzerParcelOptions & 2;
43+
44+
Parcel* p;
45+
if (resultShouldBeView) {
46+
options->extraParcels.push_back(std::make_unique<Parcel>());
47+
// held for duration of test, so that view will be valid
48+
p = options->extraParcels[options->extraParcels.size() - 1].get();
49+
} else {
50+
p = outputParcel; // directly fill out the output Parcel
51+
}
52+
auto viewify_guard = binder::impl::make_scope_guard([&]() {
53+
if (resultShouldBeView) {
54+
outputParcel->makeDangerousViewOf(p);
55+
}
56+
});
57+
58+
if (resultShouldBeRpc) {
3959
auto session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
4060
CHECK_EQ(OK, session->addNullDebuggingClient());
4161
// Set the protocol version so that we don't crash if the session

libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,9 @@ void generateSeedsFromRecording(borrowed_fd fd,
281281
// This buffer holds the bytes which will be used for fillRandomParcel API
282282
std::vector<uint8_t> fillParcelBuffer;
283283

284-
// Don't take rpc path
285-
uint8_t rpcBranch = 0;
286-
impl::writeReversedBuffer(fillParcelBuffer, rpcBranch);
284+
// Use all default options.
285+
uint8_t parcelOptions = 0;
286+
impl::writeReversedBuffer(fillParcelBuffer, parcelOptions);
287287

288288
// Implicit branch on this path -> options->writeHeader(p, provider)
289289
uint8_t writeHeaderInternal = 0;

0 commit comments

Comments
 (0)