Skip to content

Commit 916b455

Browse files
committed
fix: validate FFI round-trip lifetimes
1 parent 70ebdd0 commit 916b455

6 files changed

Lines changed: 85 additions & 38 deletions

File tree

NativeScript/ffi/shared/bridge/Callbacks.mm

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,8 @@ Function persistentEngineFunction(Runtime& runtime, const Function& function) {
390390
Function function, bool block,
391391
NativeApiCallbackThreadPolicy threadPolicy =
392392
NativeApiCallbackThreadPolicy::Default,
393-
bool bindThis = false)
393+
bool bindThis = false,
394+
uintptr_t roundTripValidationKey = 0)
394395
: runtimeOwner_(retainNativeApiRuntime(runtime)),
395396
runtime_(runtimeOwner_.get()),
396397
bridge_(std::move(bridge)),
@@ -399,7 +400,8 @@ Function persistentEngineFunction(Runtime& runtime, const Function& function) {
399400
persistentEngineFunction(runtime, function))),
400401
block_(block),
401402
threadPolicy_(threadPolicy),
402-
bindThis_(bindThis) {
403+
bindThis_(bindThis),
404+
roundTripValidationKey_(roundTripValidationKey) {
403405
closure_ = static_cast<ffi_closure*>(
404406
ffi_closure_alloc(sizeof(ffi_closure), &executable_));
405407
if (closure_ == nullptr || executable_ == nullptr ||
@@ -482,7 +484,8 @@ void retainBlockCopy(const void* blockPointer) {
482484
if (bridge_ != nullptr && runtime_ != nullptr && function_ != nullptr &&
483485
blockPointer != nullptr) {
484486
bridge_->rememberRoundTripValue(*runtime_, blockPointer,
485-
Value(*runtime_, *function_));
487+
Value(*runtime_, *function_), false,
488+
roundTripValidationKey_);
486489
}
487490
std::lock_guard<std::mutex> lock(retainedBlockCopiesMutex_);
488491
retainedBlockCopies_.push_back({blockPointer, std::move(self)});
@@ -895,6 +898,7 @@ void storeReturnValue(const Value& result, void* ret) {
895898
NativeApiCallbackThreadPolicy threadPolicy_ =
896899
NativeApiCallbackThreadPolicy::Default;
897900
bool bindThis_ = false;
901+
uintptr_t roundTripValidationKey_ = 0;
898902
ffi_closure* closure_ = nullptr;
899903
void* executable_ = nullptr;
900904
std::string blockSignature_;
@@ -2122,9 +2126,11 @@ throw JSError(
21222126

21232127
auto signature =
21242128
std::make_shared<NativeApiSignature>(std::move(*parsed));
2129+
uintptr_t roundTripValidationKey =
2130+
NativeApiBridge::callbackRoundTripValidationKey(type);
21252131
auto callback = std::make_shared<NativeApiCallback>(
21262132
runtime, bridge, std::move(signature), std::move(function), block,
2127-
threadPolicy);
2133+
threadPolicy, false, roundTripValidationKey);
21282134
if (block) {
21292135
callback->retainInitialBlockLifetime(callback);
21302136
} else {

NativeScript/ffi/shared/bridge/HostObjects.mm

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,7 @@ Array runtimeMembersArray(Runtime& runtime, Class cls, bool staticMembers) {
646646
if (object_ != nil && !ownsObject_) {
647647
[object_ retain];
648648
ownsObject_ = true;
649+
wrapperRetainedObject_ = true;
649650
}
650651
}
651652

@@ -685,6 +686,7 @@ void disownObject(id expected) {
685686
bridge_->forgetObjectExpandos(expected);
686687
}
687688
ownsObject_ = false;
689+
wrapperRetainedObject_ = false;
688690
object_ = nil;
689691
if (lifetimeState_ != nullptr) {
690692
lifetimeState_->clear();
@@ -749,6 +751,7 @@ throw JSError(runtime,
749751
// returning `this` still have a valid native object.
750752
object_ = resultObject;
751753
ownsObject_ = true;
754+
wrapperRetainedObject_ = true;
752755
if (lifetimeState_ != nullptr) {
753756
lifetimeState_->setObject(object_);
754757
}
@@ -803,6 +806,7 @@ throw JSError(runtime,
803806
// returning `this` still have a valid native object.
804807
object_ = resultObject;
805808
ownsObject_ = true;
809+
wrapperRetainedObject_ = true;
806810
if (lifetimeState_ != nullptr) {
807811
lifetimeState_->setObject(object_);
808812
}
@@ -1183,25 +1187,29 @@ Value get(Runtime& runtime, const PropNameID& name) override {
11831187

11841188
id object = self->object_;
11851189
bool ownsObject = self->ownsObject_;
1190+
bool wrapperRetainedObject = self->wrapperRetainedObject_;
11861191
if (self->bridge_ != nullptr) {
11871192
self->bridge_->forgetRoundTripValue(runtime, object);
11881193
self->bridge_->forgetObjectExpandos(object);
11891194
}
11901195
self->object_ = nil;
11911196
self->ownsObject_ = false;
1197+
self->wrapperRetainedObject_ = false;
11921198
if (self->lifetimeState_ != nullptr) {
11931199
self->lifetimeState_->clear();
11941200
}
11951201
self->consumed_ = true;
1202+
const bool releasePreviousOwnership =
1203+
ownsObject && (!retained || wrapperRetainedObject);
11961204
try {
11971205
Value result =
11981206
makeNativeObjectValue(runtime, self->bridge_, object, retained);
1199-
if (!retained && ownsObject) {
1207+
if (releasePreviousOwnership) {
12001208
[object release];
12011209
}
12021210
return result;
12031211
} catch (...) {
1204-
if (!retained && ownsObject) {
1212+
if (releasePreviousOwnership) {
12051213
[object release];
12061214
}
12071215
throw;
@@ -1517,6 +1525,7 @@ throw JSError(
15171525
std::shared_ptr<NativeApiBridge> bridge_;
15181526
id object_ = nil;
15191527
bool ownsObject_ = false;
1528+
bool wrapperRetainedObject_ = false;
15201529
bool consumed_ = false;
15211530
std::shared_ptr<NativeApiObjectLifetimeState> lifetimeState_;
15221531
};
@@ -1865,6 +1874,15 @@ Value makeNativeObjectValue(Runtime& runtime,
18651874
if (!prototypeValue.isObject()) {
18661875
prototypeValue = bridge->findClassPrototype(runtime, object_getClass(object));
18671876
}
1877+
if (!prototypeValue.isObject()) {
1878+
Value classWrapper = makeNativeClassValue(
1879+
runtime, bridge,
1880+
nativeApiSymbolForRuntimeClass(bridge, object_getClass(object)));
1881+
if (classWrapper.isObject()) {
1882+
prototypeValue =
1883+
classWrapper.asObject(runtime).getProperty(runtime, "prototype");
1884+
}
1885+
}
18681886
if (prototypeValue.isObject()) {
18691887
Object prototype = prototypeValue.asObject(runtime);
18701888
SetNativeApiObjectPrototype(runtime, result, prototype);

NativeScript/ffi/shared/bridge/ObjCBridge.mm

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ inline uintptr_t normalizeRuntimePointer(uintptr_t pointer) {
565565
std::shared_ptr<Value> value;
566566
bool stringLikeNative = false;
567567
bool persistBeyondFrame = true;
568-
uintptr_t objectClassKey = 0;
568+
uintptr_t validationKey = 0;
569569
};
570570
using NativeApiRoundTripReleaseList =
571571
std::vector<NativeApiRoundTripValue>;
@@ -657,9 +657,20 @@ explicit NativeApiBridge(const NativeApiConfig& config)
657657
return it != functionSymbolsByName_.end() ? &it->second : nullptr;
658658
}
659659

660+
static uintptr_t callbackRoundTripValidationKey(
661+
const NativeApiType& type) {
662+
if (type.signatureOffset == 0 ||
663+
type.signatureOffset == MD_SECTION_OFFSET_NULL) {
664+
return 0;
665+
}
666+
return (static_cast<uintptr_t>(type.signatureOffset) << 8) |
667+
(static_cast<uintptr_t>(type.kind) & 0xff);
668+
}
669+
660670
void rememberRoundTripValue(Runtime& runtime, const void* native,
661671
const Value& value,
662-
bool stringLikeNative = false) {
672+
bool stringLikeNative = false,
673+
uintptr_t validationKey = 0) {
663674
if (native == nullptr) {
664675
return;
665676
}
@@ -672,7 +683,7 @@ void rememberRoundTripValue(Runtime& runtime, const void* native,
672683
roundTripValues_, key,
673684
NativeApiRoundTripValue{
674685
std::make_shared<Value>(runtime, value), stringLikeNative, true,
675-
0},
686+
validationKey},
676687
releaseAfterUnlock);
677688
roundTripValuesGeneration_.fetch_add(1, std::memory_order_release);
678689
}
@@ -685,7 +696,7 @@ void rememberScopedRoundTripValue(Runtime& runtime, const void* native,
685696
const Value& value,
686697
bool stringLikeNative = false,
687698
bool persistBeyondFrame = true) {
688-
rememberScopedRoundTripValueWithClassKey(
699+
rememberScopedRoundTripValueWithValidationKey(
689700
runtime, native, value, stringLikeNative, persistBeyondFrame,
690701
nativeObjectClassKey(native));
691702
}
@@ -694,26 +705,26 @@ void rememberScopedRawRoundTripValue(Runtime& runtime, const void* native,
694705
const Value& value,
695706
bool stringLikeNative = false,
696707
bool persistBeyondFrame = true) {
697-
rememberScopedRoundTripValueWithClassKey(runtime, native, value,
698-
stringLikeNative,
699-
persistBeyondFrame, 0);
708+
rememberScopedRoundTripValueWithValidationKey(runtime, native, value,
709+
stringLikeNative,
710+
persistBeyondFrame, 0);
700711
}
701712

702-
void rememberScopedRoundTripValueWithClassKey(Runtime& runtime,
703-
const void* native,
704-
const Value& value,
705-
bool stringLikeNative,
706-
bool persistBeyondFrame,
707-
uintptr_t objectClassKey) {
713+
void rememberScopedRoundTripValueWithValidationKey(Runtime& runtime,
714+
const void* native,
715+
const Value& value,
716+
bool stringLikeNative,
717+
bool persistBeyondFrame,
718+
uintptr_t validationKey) {
708719
if (native == nullptr) {
709720
return;
710721
}
711722
uintptr_t key =
712723
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
713724
NativeApiRoundTripValue entry{
714725
std::make_shared<Value>(runtime, value), stringLikeNative,
715-
persistBeyondFrame, objectClassKey};
716-
NativeApiRoundTripReleaseList releaseAfterUnlock;
726+
persistBeyondFrame, validationKey};
727+
NativeApiRoundTripReleaseList releaseAfterUnlock;
717728
{
718729
std::lock_guard<std::mutex> lock(roundTripValuesMutex_);
719730
auto framesIt =
@@ -732,7 +743,8 @@ void rememberScopedRoundTripValueWithClassKey(Runtime& runtime,
732743

733744
Value findRoundTripValue(Runtime& runtime, const void* native,
734745
bool* stringLikeNative = nullptr,
735-
bool nativeIsObject = false) {
746+
bool nativeIsObject = false,
747+
uintptr_t validationKey = 0) {
736748
if (stringLikeNative != nullptr) {
737749
*stringLikeNative = false;
738750
}
@@ -741,13 +753,18 @@ Value findRoundTripValue(Runtime& runtime, const void* native,
741753
}
742754
uintptr_t key =
743755
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
756+
const uintptr_t expectedValidationKey =
757+
validationKey != 0
758+
? validationKey
759+
: (nativeIsObject ? nativeObjectClassKey(native) : 0);
744760
struct RoundTripCacheEntry {
745761
const NativeApiBridge* bridge = nullptr;
746762
uintptr_t key = 0;
747763
uint64_t generation = 0;
748764
std::weak_ptr<Value> value;
749765
bool miss = false;
750766
bool stringLikeNative = false;
767+
uintptr_t validationKey = 0;
751768
};
752769
static thread_local RoundTripCacheEntry cache[4];
753770
const uint64_t generation =
@@ -757,6 +774,9 @@ Value findRoundTripValue(Runtime& runtime, const void* native,
757774
RoundTripCacheEntry& entry = cache[(firstSlot + i) & 3];
758775
if (entry.bridge == this && entry.key == key &&
759776
entry.generation == generation) {
777+
if (entry.validationKey != expectedValidationKey) {
778+
break;
779+
}
760780
if (entry.miss) {
761781
return Value::undefined();
762782
}
@@ -782,11 +802,8 @@ Value findRoundTripValue(Runtime& runtime, const void* native,
782802
if (it == map.end() || it->second.value == nullptr) {
783803
return nullptr;
784804
}
785-
if (it->second.objectClassKey != 0) {
786-
if (!nativeIsObject ||
787-
it->second.objectClassKey != nativeObjectClassKey(native)) {
788-
return nullptr;
789-
}
805+
if (it->second.validationKey != expectedValidationKey) {
806+
return nullptr;
790807
}
791808
return &it->second;
792809
};
@@ -809,14 +826,15 @@ Value findRoundTripValue(Runtime& runtime, const void* native,
809826
entry = findEntry(recentRoundTripValues_);
810827
}
811828
if (entry == nullptr) {
812-
cache[firstSlot] =
813-
RoundTripCacheEntry{this, key, generation, {}, true, false};
829+
cache[firstSlot] = RoundTripCacheEntry{
830+
this, key, generation, {}, true, false, expectedValidationKey};
814831
return Value::undefined();
815832
}
816833
storedValue = entry->value;
817834
cachedStringLike = entry->stringLikeNative;
818835
cache[firstSlot] = RoundTripCacheEntry{
819-
this, key, generation, storedValue, false, cachedStringLike};
836+
this, key, generation, storedValue, false, cachedStringLike,
837+
entry->validationKey};
820838
}
821839
if (stringLikeNative != nullptr) {
822840
*stringLikeNative = cachedStringLike;

NativeScript/ffi/shared/bridge/TypeConv.mm

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,12 +842,16 @@ throw JSError(
842842
createEngineCallback(runtime, bridge, type, object.asFunction(runtime),
843843
type.kind == metagen::mdTypeBlock, threadPolicy);
844844
void* pointer = callback->functionPointer();
845+
uintptr_t roundTripValidationKey =
846+
NativeApiBridge::callbackRoundTripValidationKey(type);
845847
if (type.kind == metagen::mdTypeBlock) {
846848
frame.addObject(static_cast<id>(pointer));
847849
frame.addLifetime(callback);
848-
bridge->rememberRoundTripValue(runtime, pointer, value);
850+
bridge->rememberRoundTripValue(runtime, pointer, value, false,
851+
roundTripValidationKey);
849852
} else {
850-
bridge->rememberRoundTripValue(runtime, pointer, value);
853+
bridge->rememberRoundTripValue(runtime, pointer, value, false,
854+
roundTripValidationKey);
851855
}
852856
try {
853857
object.setProperty(runtime, "__nativeApiPointerObject",
@@ -1077,7 +1081,9 @@ throw JSError(runtime,
10771081
if (pointer == nullptr) {
10781082
return Value::null();
10791083
}
1080-
Value roundTrip = bridge->findRoundTripValue(runtime, pointer);
1084+
Value roundTrip = bridge->findRoundTripValue(
1085+
runtime, pointer, nullptr, false,
1086+
NativeApiBridge::callbackRoundTripValidationKey(type));
10811087
if (!roundTrip.isUndefined()) {
10821088
return roundTrip;
10831089
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
id functionReturnsNSRetained() NS_RETURNS_RETAINED;
2-
id functionReturnsCFRetained() CF_RETURNS_RETAINED;
2+
id functionReturnsCFRetained() CF_RETURNS_RETAINED NS_RETURNS_RETAINED;
33

44
CF_IMPLICIT_BRIDGING_ENABLED
55
CFTypeRef functionImplicitCreate();
66
CF_IMPLICIT_BRIDGING_DISABLED
77

8-
id functionExplicitCreateNSObject();
8+
id functionExplicitCreateNSObject() NS_RETURNS_RETAINED;
99

1010
@interface TNSReturnsRetained : NSObject
1111
+ (id)methodReturnsNSRetained NS_RETURNS_RETAINED;
12-
+ (id)methodReturnsCFRetained CF_RETURNS_RETAINED;
12+
+ (id)methodReturnsCFRetained CF_RETURNS_RETAINED NS_RETURNS_RETAINED;
1313
+ (id)newNSObjectMethod;
1414
@end

test/runtime/runner/app/tests/ApiTests.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,8 +795,7 @@ describe(module.id, function () {
795795
expect(functionImplicitCreate().retainCount()).toBe(1);
796796

797797
var obj = functionExplicitCreateNSObject();
798-
expect(obj.retainCount()).toBe(2);
799-
CFRelease(obj);
798+
expect(obj.retainCount()).toBe(1);
800799

801800
expect(TNSReturnsRetained.methodReturnsNSRetained().retainCount()).toBe(1);
802801
expect(TNSReturnsRetained.methodReturnsCFRetained().retainCount()).toBe(1);

0 commit comments

Comments
 (0)