From bd226519ad3562f58fa7bf0ba5cf74f82aa4e8d0 Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 4 Oct 2026 01:19:35 -0500 Subject: [PATCH 1/6] Use pre-built object files for tests/dynamic_code_loading/ Check in NaCl object files built from the assembly "template" files using a non-Saigo toolchain. This lets run_dynamic_modify_test work, which I want for attempting a fix for this feature on Mac. --- .../dynamic_code_loading/dynamic_load_test.c | 2 +- tests/dynamic_code_loading/nacl.scons | 27 +++++++++++++----- tests/dynamic_code_loading/templates_arm.o | Bin 0 -> 2844 bytes tests/dynamic_code_loading/templates_x86-32.o | Bin 0 -> 4472 bytes tests/dynamic_code_loading/templates_x86-64.o | Bin 0 -> 4224 bytes 5 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 tests/dynamic_code_loading/templates_arm.o create mode 100644 tests/dynamic_code_loading/templates_x86-32.o create mode 100644 tests/dynamic_code_loading/templates_x86-64.o diff --git a/tests/dynamic_code_loading/dynamic_load_test.c b/tests/dynamic_code_loading/dynamic_load_test.c index b9ef602882..0bd961b0a6 100644 --- a/tests/dynamic_code_loading/dynamic_load_test.c +++ b/tests/dynamic_code_loading/dynamic_load_test.c @@ -21,7 +21,7 @@ #if defined(__x86_64__) /* On x86-64, template functions do not fit in 32-byte buffers */ -#define BUF_SIZE 128 +#define BUF_SIZE 64 #elif defined(__i386__) || defined(__arm__) #define BUF_SIZE 32 #else diff --git a/tests/dynamic_code_loading/nacl.scons b/tests/dynamic_code_loading/nacl.scons index 8a5490c000..8d4a32a41f 100644 --- a/tests/dynamic_code_loading/nacl.scons +++ b/tests/dynamic_code_loading/nacl.scons @@ -30,11 +30,6 @@ if env.Bit('build_mips32'): # See http://code.google.com/p/nativeclient/issues/detail?id=1112 is_broken = not env.Bit('nacl_static_link') -# there is fair amount of assembly code in these tests -asm_env = env.Clone() -if env.Bit('bitcode'): - asm_env.PNaClForceNative() - asm_env.AddBiasForPNaCl() if env.Bit('bitcode'): # NOTE: we cannot use PNaClForceNative here as we want the # the .c files to actually go to bc files - but this is not @@ -58,7 +53,25 @@ def GetTemplate(env): else: return 'templates_x86.S' -template_obj = asm_env.ComponentObject(GetTemplate(env)) +# These are built with the default toolchain using Chromium tools. The Saigo +# assembler heavily modifies the code making it not work with dynamic_modify_test. +def GetPrebuiltTemplate(env): + if env.Bit('build_arm'): + return 'templates_arm.o' + if env.Bit('build_x86_32'): + return 'templates_x86-32.o' + if env.Bit('build_x86_64'): + return 'templates_x86-64.o' + +if env.Bit('saigo'): + template_obj = File(GetPrebuiltTemplate(env)) +else: + # there is fair amount of assembly code in these tests + asm_env = env.Clone() + if env.Bit('bitcode'): + asm_env.PNaClForceNative() + asm_env.AddBiasForPNaCl() + template_obj = asm_env.ComponentObject(GetTemplate(env)) dynamic_load_test_nexe = env.ComponentProgram( env.ProgramNameForNmf('dynamic_load_test'), @@ -179,4 +192,4 @@ node = env.CommandSelLdrTestNacl('dynamic_modify_test.out', # shared memory segment. It does not know to flush its code # translation cache. env.AddNodeToTestSuite(node, test_suites, 'run_dynamic_modify_test', - is_broken=is_broken or env.Bit('saigo') or env.IsRunningUnderValgrind()) + is_broken=is_broken or env.IsRunningUnderValgrind()) diff --git a/tests/dynamic_code_loading/templates_arm.o b/tests/dynamic_code_loading/templates_arm.o new file mode 100644 index 0000000000000000000000000000000000000000..675cf39f2a9e25c518b4f3f525d85a99d8d62667 GIT binary patch literal 2844 zcma)8&x;&I82x%?vOkg~jKN4Co4CRn*AP27>_H)rT{VgjMMN-OGO3xV%udqNvvk)O zJ-LD)-Xi7?*n>wA4<5yXXVDNmcu}&>L5Sw=L9)JA+tpLk+dIq$)%Cr4U%jfYs=B&A zzI^4iAPB@+AS*ILA{XZ^b7<98E3zQ-mY;PUWP3+Y?#uQtII=aowDa@u^zQkcv-RN5 z$Gf|KlVjN-e|SId`rWUCKlw@C-}T%5HKpZ`5l`wzyBuWznZO&)2)d-smulQfrRNtJ)@eaO|n;SI4{vrX!M6hLV9gwl!VPRl`!rb9a1}7 zfBB7&;^F!`Z-&a4q}v=AjgU^#A8c~y1Fb5RF|KN7J!-1T%m~1%%B;<4A#a^6^r%?= zdd^oYKbplJ&S|hb)A#a*nk>@KUk0w?$f`0o0`RH`e93F6BcIoSm$EkE_X{`$Ui0`b z3iun~s~-Og{Ocb66~4=d`o9{O1My9q_jvKP=#xs>p91{{rv>4_^hI z!<=Z$PnO%VtORFVC$WkeHwV4VMxxCiiT5>bMyjP9X>$ZW&~dxal4#qx zr(<(4Q_?T)#|L*ECvESI$fUxDMa@LTt&PTwesWtS?Nkcbk7=sbt+AlI!A5oC4^-4` zH(LF+79Uu4Jo{cZMQkV5ZLHNnX_p6;sCA=Acd#*u?sQXZ(ge*(p%aul11!!IvissD zXtq=oHH=C+y13tNl3v6loqn7e6`Luh*D47uQ64|(rBf^mnNrEkvv9~u6|=)(mE}pi zk1Hs=e;1F6@ZW>@B5_^h-3;^YICVnoop$Oe02t$zvicnAMToqb;VZ%hph6cOmv7YU<$%w3^g^9))_i=B(Yrv(ARhu$0!G#nadO37xsN{`{zS z9)-2jd*ErI*^b4U`SDv?;bdTp!#3r{oN4?%LW@IL`xkN9Hd^fDxpr<$5=WtX%|`4! iTpa6cNJ~Tj+bI0M!7;My!kn3_ueX4oq|dwtr}sa6YmW5* literal 0 HcmV?d00001 diff --git a/tests/dynamic_code_loading/templates_x86-32.o b/tests/dynamic_code_loading/templates_x86-32.o new file mode 100644 index 0000000000000000000000000000000000000000..d34f76f25ffb7f6ae1c569b62f8b958a4322e6dd GIT binary patch literal 4472 zcma)>#C$ggjHz!R_WFfs}h2U3Kd~?GJAJ3>FjKnnY9%K z15zY{qz{4!LgRx7KKc}?!j|Gg1%2>Eq!BijVrB6`=$p#=-8+AB=I+d7FPXjPcfWJa zJ#+7!$?kh|^Us$`C5l^#%Ctj7K{-^8scx3`(p|Bu>NkI$Bs%;4c7M1%xc^eB-Trqt z9Byv!&(*h}hmi+Tff3y9L%+l)xG&|V^55B|{V9t`g{5IPy{NvMNd4pLwbXj(uv7l; z+3ORn-!O0e3fA{G%zsCpP-8X!;P7BNecO`4{rUJvygyoGh-&}Fi{F$Hn-8L~OltkU zx$ye2X*S4ArQ>7mIvZrB3RO;QkeMp9Kws;9G)eed>#-najxI|wqCyiIHNvb_|Lt~Q zE>hJGys%%2El)VrKwJo@Dy@bYm{eW#eKvcs`6y)hEXGe<4R#+_+$xcA5dGeG5dADR zU>5xzTrwU{DBX?g$%vyznko=06BvwICF*C;^L#I}d@?r9_cUyoH_lg!4UEYq{bZBp zflJaae4i*;)hI1lD=UY#kf4rB&T$VenGbIhRU6ND0>uvG%|=b;$!clhTZvw#7humM#g$1Un!K+P$&*ou#w1_V_%gW0 z^zSsj0j@LszsQ!|epGf4b627<$eu?QraGmiZQ$&LC zNsR?~nd$4`RmNX}FEYNS@lEiDOy7eS+Xmwq@E45FXxs*W#q>|WJ;vXHzh`_)<7vDj zeqj1(aG!BYzC&;(WRv@~uJILch3T|#>h%1n`4`VDC~I!v)mP4jxZG{87`AV8&QWZd z=bK%J@ZUgquIU(R*b_n5b4LbO9Mcj;BnF}l4usL{x)!C#5N<;o@q`L)3pdDDR9sk> z8*~i6yCOWpcKyKUcy`N1c~R9lYJPmX)3BSiXk^F7YDV2|$IUN2EoMZwZQpe4mMa=q zwY&w$c2e1voWLjfp=i-FU2Dl`cD(bZ*YHVW?z3oG=Tc6|fbz24w@_EZusRJv9F&^g zOWSco3kSm!E!)Q-DY%m&*MiKgE^cRxv?G26IW5z1jKK6-LO*YDk<6ngPY1`(bb~R< zvoc&WGN+%k%o4L)#>^DgsXb(w8KWba&MY4{Zzoz|hR5hUp(oGT5RSn6K|RJw?OK7| zaVck9&61}@d2?gOnAh-E96QL(pwz+|LXX`lNqMP!mM7$Ua%3HH#*1Q-WzUP{>yYOY zu0x?(SVQU#sW?xno+Z<531hL-bsMH<3*Ttk9$xP3Z1`l2;S^Rkj$^!8V`VL_)iHN3 zb;Kv9(ow(sLUVmpZaReT1QYlXazgg|h~Aa`f$;m_1bzrymHZIVo3cL~=@p{SBtK5H PCHo^phwy!4VwUKC+_CLB literal 0 HcmV?d00001 diff --git a/tests/dynamic_code_loading/templates_x86-64.o b/tests/dynamic_code_loading/templates_x86-64.o new file mode 100644 index 0000000000000000000000000000000000000000..89e0dc72bf5a8a9d1170261141b234ef07d5ad93 GIT binary patch literal 4224 zcmbuCO^6&t6vt~a`EX6ht{z14;f^1OQ5abQAqSBj!X}Yy77-C7fu?u5X10@_o~64d zyL!ol1SEJ_Fu5h*EndvU%N`O~*h3ED2VOk5ft4VM2$F!tLxlCc>Q^(}ud8QgO~FjR z`qlrvdiB*)bAEE_=$2AR=u#5zihfN~Ar{Jg+ft?_M#Oe8Iq}Ph&o7N5shz4_ytj6; z^rW_aZ*<%e4;1`oWgW`#JIg<96=HN;G*qdMj$cwyhWGz<%;ewiva!fQKUjl~7YjQW zF2BDvD8J(<7K_V9?St~a5BY!BT~YH?^i?=KKL0B?|M@aG@pu#KXY$|7`u{Kghfrw$ zo<1l&7YAck!Q;p?9 zEUL(UdWPyR((^6i zsn>#P=~BP9H|QVM#MuAhXwt#MJanZ1s|UO2ryIsho4RSud;-B|A{L9o#-B|QyM+NPG#je;<6e|Puere z#tc!-Z<*M~K6aGbJEiW&ApEJ`c;pBc{Jkyvz9W@SebYzG1vp zz~3jn#Psz7{wZ<(J{QxqBU81h_l`{~YjXO-4?a$+t{2UTK+fCBIMZp(*hcfPjK@QIOPF>p67|RyaV`(=!L0x2OwhUZGLaCfvGKh;?lwXop z5V!59Gbcma3!>O=hhEcz^eF3s)?)uw+w~frbj|*{)sT144vJIHHX6KJUgY>*GmtLK zTG0fUMJn2?A4dYeRBa}7g8Ho8XovGo=thFo)0!Elel{cO0j2VSbC8#7*W0cX>56L2 zuBGSuvI!d_lua*!O)_94gQNo@H#;z#A;RW#17g-4-?w8YY)Zc0bdo8>pg3*Veohb% zkzU;(HAK#yG-HY;$&itBp6rkrBhr@4M#ke7_Ptfrpvd==DAO_F&5QR%EO_66S7z@% zvJW8?c&)4Z*uVI%6v{D0`+9v(IZYh(yWz8%$k)GOdVSxymD0ZewvMm=T4Nl{LGzdC z{fiv$PMY(4bo_Kam6ZN7u(9y*UQ4hV?<-*I_;-kN{c1}88rXMfygMjQuHTo+AMf%y R{^=CQJD%p(QXI#m`F|9jj+Foa literal 0 HcmV?d00001 From cfea6c796f0b85bdf17726658cb49e61a9beec99 Mon Sep 17 00:00:00 2001 From: slipher Date: Sat, 3 Oct 2026 05:10:26 -0500 Subject: [PATCH 2/6] Store Rosetta emulation detection result --- src/trusted/debug_stub/nacl_debug.cc | 21 +-------------------- src/trusted/service_runtime/sel_ldr.c | 23 +++++++++++++++++++++++ src/trusted/service_runtime/sel_ldr.h | 4 ++++ 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/trusted/debug_stub/nacl_debug.cc b/src/trusted/debug_stub/nacl_debug.cc index 324d523e1d..e10a8647d9 100644 --- a/src/trusted/debug_stub/nacl_debug.cc +++ b/src/trusted/debug_stub/nacl_debug.cc @@ -28,10 +28,6 @@ #include "native_client/src/trusted/service_runtime/sel_ldr.h" #include "native_client/src/trusted/service_runtime/thread_suspension.h" -#if NACL_OSX -# include -#endif - using port::IPlatform; using port::Thread; using port::ITransport; @@ -120,28 +116,13 @@ static const struct NaClDebugCallbacks debug_callbacks = { ProcessExitHook, }; -#if NACL_OSX -// From https://developer.apple.com/documentation/apple-silicon/about-the-rosetta-translation-environment -static int processIsTranslated() { - int ret = 0; - size_t size = sizeof(ret); - if (sysctlbyname("sysctl.proc_translated", &ret, &size, NULL, 0) == -1) - { - if (errno == ENOENT) - return 0; - return -1; - } - return ret; -} -#endif - /* * This function is implemented for the service runtime. The service runtime * declares the function so it does not need to be declared in our header. */ int NaClDebugInit(struct NaClApp *nap) { #if NACL_OSX - if (processIsTranslated() != 0) { + if (nap->in_emulator) { // Thread suspension facilities don't work NaClLog(LOG_ERROR, "NaCl debugging not available under Rosetta translation\n"); return 0; diff --git a/src/trusted/service_runtime/sel_ldr.c b/src/trusted/service_runtime/sel_ldr.c index b78ec9f784..5f44bf574d 100644 --- a/src/trusted/service_runtime/sel_ldr.c +++ b/src/trusted/service_runtime/sel_ldr.c @@ -15,6 +15,11 @@ #include #endif +#if NACL_OSX +#include +#include +#endif + #include "native_client/src/include/portability.h" #include "native_client/src/include/portability_io.h" #include "native_client/src/include/portability_string.h" @@ -81,6 +86,21 @@ static int CheckPageSize(size_t size) { #endif } +#if NACL_OSX +// https://developer.apple.com/documentation/apple-silicon/about-the-rosetta-translation-environment +static int ProcessIsTranslated(void) { + int ret = 0; + size_t size = sizeof(ret); + if (sysctlbyname("sysctl.proc_translated", &ret, &size, NULL, 0) == -1) + { + if (errno == ENOENT) + return 0; + NaClLog(LOG_FATAL, "Failed to retrieve sysctl.proc_translated\n"); + } + return ret; +} +#endif + int NaClAppWithEmptySyscallTableCtor(struct NaClApp *nap) { struct NaClDescEffectorLdr *effp; int i; @@ -270,6 +290,9 @@ int NaClAppWithEmptySyscallTableCtor(struct NaClApp *nap) { nap->faulted_thread_fd_write = -1; #endif +#if NACL_OSX + nap->in_emulator = ProcessIsTranslated(); +#endif #if NACL_LINUX || NACL_OSX /* diff --git a/src/trusted/service_runtime/sel_ldr.h b/src/trusted/service_runtime/sel_ldr.h index ce3b3497e9..193bd2a781 100644 --- a/src/trusted/service_runtime/sel_ldr.h +++ b/src/trusted/service_runtime/sel_ldr.h @@ -380,6 +380,10 @@ struct NaClApp { */ int sc_nprocessors_onln; +#if NACL_OSX + int in_emulator; +#endif + size_t page_size; const struct NaClValidatorInterface *validator; From 4484dd6099639a37f5dc5935e5a787d1f6f9522c Mon Sep 17 00:00:00 2001 From: slipher Date: Sun, 4 Oct 2026 02:08:09 -0500 Subject: [PATCH 3/6] Minor cleanup in NaClFlushCacheForDoublyMappedCode __builtin___clear_cache can now be used on Mac. --- src/include/concurrency_ops.h | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/include/concurrency_ops.h b/src/include/concurrency_ops.h index 2ac4b913af..b35e5889ce 100644 --- a/src/include/concurrency_ops.h +++ b/src/include/concurrency_ops.h @@ -58,25 +58,19 @@ static INLINE void NaClWriteMemoryBarrier(void) { static INLINE void NaClFlushCacheForDoublyMappedCode(uint8_t *writable_addr, uint8_t *executable_addr, size_t size) { -#if NACL_ARCH(NACL_BUILD_ARCH) == NACL_x86 +#if NACL_WINDOWS /* - * Clearing the icache explicitly is not necessary on x86. We could - * call gcc's __builtin___clear_cache() on x86, where it is a no-op, - * except that it is not available in Mac OS X's old version of gcc. - * We simply prevent the compiler from moving loads or stores around + * Clearing the icache explicitly is not necessary on x86. The compiler + * only must be prevented from moving loads or stores around * this function. */ NACL_UNUSED_PARAMETER(writable_addr); NACL_UNUSED_PARAMETER(executable_addr); NACL_UNUSED_PARAMETER(size); -#if NACL_WINDOWS _ReadWriteBarrier(); -#else - __asm__ __volatile__("" : : : "memory"); -#endif #elif defined(__GNUC__) /* - * __clear_cache() does two things: + * For ARM __clear_cache() does two things: * * 1) It flushes the write buffer for the address range. * We need to do this for writable_addr. From da1b52a16230d5ef1373c8ae93fe7acd1a964b43 Mon Sep 17 00:00:00 2001 From: slipher Date: Mon, 5 Oct 2026 05:04:42 -0500 Subject: [PATCH 4/6] Move NaClCopyCode function to nacl_text.c --- src/trusted/service_runtime/nacl_text.c | 21 +++++++++++++++++ src/trusted/service_runtime/sel_ldr.h | 9 ++------ .../service_runtime/sel_validate_image.c | 23 +------------------ 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/trusted/service_runtime/nacl_text.c b/src/trusted/service_runtime/nacl_text.c index 95d9047014..13190fbdad 100644 --- a/src/trusted/service_runtime/nacl_text.c +++ b/src/trusted/service_runtime/nacl_text.c @@ -96,6 +96,27 @@ static struct NaClDesc *MakeImcShmDesc(uintptr_t size) { return &shm->base; } +static int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, + uint8_t *data_old, uint8_t *data_new, + size_t size) { + int status; + status = NaClValidateStatus(nap->validator->CopyCode( + guest_addr, data_old, data_new, size, + nap->cpu_features, + NaClCopyInstruction)); + /* + * Flush the processor's instruction cache. This is not necessary + * for security, because any old cached instructions will just be + * safe halt instructions. It is only necessary to ensure that + * untrusted code runs correctly when it tries to execute the + * dynamically-loaded code. + */ + NaClFlushCacheForDoublyMappedCode(data_old, + (uint8_t *) guest_addr, + size); + return status; +} + NaClErrorCode NaClMakeDynamicTextShared(struct NaClApp *nap) { uintptr_t dynamic_text_size; uintptr_t shm_vaddr_base; diff --git a/src/trusted/service_runtime/sel_ldr.h b/src/trusted/service_runtime/sel_ldr.h index 193bd2a781..486b65fefa 100644 --- a/src/trusted/service_runtime/sel_ldr.h +++ b/src/trusted/service_runtime/sel_ldr.h @@ -461,6 +461,8 @@ NaClErrorCode NaClAppLoadFileDynamically( struct NaClDesc *ndp, struct NaClValidationMetadata *metadata) NACL_WUR; +int NaClValidateStatus(NaClValidationStatus status); + int NaClValidateCode(struct NaClApp *nap, uintptr_t guest_addr, uint8_t *data, @@ -477,13 +479,6 @@ int NaClValidateCodeReplacement(struct NaClApp *nap, uint8_t *data_new, size_t size); -/* - * Copies code from data_new to data_old in a thread-safe way. - */ -int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, - uint8_t *data_old, uint8_t *data_new, - size_t size); - /* * Copies an instruction in a thread-safe way. Used by validators. */ diff --git a/src/trusted/service_runtime/sel_validate_image.c b/src/trusted/service_runtime/sel_validate_image.c index 389ae8bbe8..df0bbceecb 100644 --- a/src/trusted/service_runtime/sel_validate_image.c +++ b/src/trusted/service_runtime/sel_validate_image.c @@ -13,7 +13,7 @@ const size_t kMinimumCachedCodeSize = 40000; /* Translate validation status to values wanted by sel_ldr. */ -static int NaClValidateStatus(NaClValidationStatus status) { +int NaClValidateStatus(NaClValidationStatus status) { switch (status) { case NaClValidationSucceeded: return LOAD_OK; @@ -98,27 +98,6 @@ int NaClValidateCodeReplacement(struct NaClApp *nap, uintptr_t guest_addr, guest_addr, data_old, data_new, size, nap->cpu_features)); } -int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, - uint8_t *data_old, uint8_t *data_new, - size_t size) { - int status; - status = NaClValidateStatus(nap->validator->CopyCode( - guest_addr, data_old, data_new, size, - nap->cpu_features, - NaClCopyInstruction)); - /* - * Flush the processor's instruction cache. This is not necessary - * for security, because any old cached instructions will just be - * safe halt instructions. It is only necessary to ensure that - * untrusted code runs correctly when it tries to execute the - * dynamically-loaded code. - */ - NaClFlushCacheForDoublyMappedCode(data_old, - (uint8_t *) guest_addr, - size); - return status; -} - NaClErrorCode NaClValidateImage(struct NaClApp *nap) { uintptr_t memp; uintptr_t endp; From 4f656a3bffe5b5d6822dcaa1f9ae99b7ad54a343 Mon Sep 17 00:00:00 2001 From: slipher Date: Mon, 5 Oct 2026 06:30:25 -0500 Subject: [PATCH 5/6] SysDyncodeModify: fix wrong instruction cache flush range guest_addr was the in-sandbox address of the code but it needs to be the real system mapped-as-executable address. --- src/trusted/service_runtime/nacl_text.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/trusted/service_runtime/nacl_text.c b/src/trusted/service_runtime/nacl_text.c index 13190fbdad..54a1c688d8 100644 --- a/src/trusted/service_runtime/nacl_text.c +++ b/src/trusted/service_runtime/nacl_text.c @@ -97,11 +97,12 @@ static struct NaClDesc *MakeImcShmDesc(uintptr_t size) { } static int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, - uint8_t *data_old, uint8_t *data_new, + uint8_t *exec_addr, + uint8_t *write_addr, uint8_t *replacement_addr, size_t size) { int status; status = NaClValidateStatus(nap->validator->CopyCode( - guest_addr, data_old, data_new, size, + guest_addr, write_addr, replacement_addr, size, nap->cpu_features, NaClCopyInstruction)); /* @@ -111,8 +112,8 @@ static int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, * untrusted code runs correctly when it tries to execute the * dynamically-loaded code. */ - NaClFlushCacheForDoublyMappedCode(data_old, - (uint8_t *) guest_addr, + NaClFlushCacheForDoublyMappedCode(write_addr, + exec_addr, size); return status; } @@ -942,7 +943,8 @@ int32_t NaClSysDyncodeModify(struct NaClAppThread *natp, goto cleanup_unlock; } - if (LOAD_OK != NaClCopyCode(nap, dest, mapped_addr, code_copy, size)) { + if (LOAD_OK != NaClCopyCode(nap, dest, (uint8_t *) dest_addr, + mapped_addr, code_copy, size)) { NaClLog(1, "NaClSysDyncodeModify: Copying of replacement code failed\n"); retval = -NACL_ABI_EINVAL; goto cleanup_unlock; From d686006a88d0c781d39daed99964b9d18a9f6725 Mon Sep 17 00:00:00 2001 From: slipher Date: Mon, 5 Oct 2026 23:50:41 -0500 Subject: [PATCH 6/6] Fix dyncode syscalls under Rosetta translation Once some thread has started executing code on a page, Rosetta apparently assumes that the code will not be modified until the PROT_EXEC bit is removed. The dyncode syscalls violate that assumption Caveat: dyncode_modify does not sync threads so this means PROT_EXEC can be toggled for code in use, which might make it crash (not tested). --- src/include/concurrency_ops.h | 4 +++ src/trusted/service_runtime/nacl_text.c | 40 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/include/concurrency_ops.h b/src/include/concurrency_ops.h index b35e5889ce..b6dc59a029 100644 --- a/src/include/concurrency_ops.h +++ b/src/include/concurrency_ops.h @@ -55,6 +55,10 @@ static INLINE void NaClWriteMemoryBarrier(void) { #endif +/* + * If it needs to work on Rosetta, RosettaFlushInstructionCache is additionally + * needed. + */ static INLINE void NaClFlushCacheForDoublyMappedCode(uint8_t *writable_addr, uint8_t *executable_addr, size_t size) { diff --git a/src/trusted/service_runtime/nacl_text.c b/src/trusted/service_runtime/nacl_text.c index 54a1c688d8..29e643a487 100644 --- a/src/trusted/service_runtime/nacl_text.c +++ b/src/trusted/service_runtime/nacl_text.c @@ -31,6 +31,7 @@ #include "native_client/src/trusted/service_runtime/thread_suspension.h" #if NACL_OSX +#include #include "native_client/src/trusted/desc/osx/nacl_desc_imc_shm_mach.h" #endif @@ -96,6 +97,39 @@ static struct NaClDesc *MakeImcShmDesc(uintptr_t size) { return &shm->base; } +/* + * Toggling PROT_EXEC seems to be the only way to make Rosetta re-translate + * pages that have been executed previously. Officially recommended methods + * like sys_icache_invalidate don't help. Better hope no untrusted code + * is executing there... + * The memory range's protection is assumed to start as exec+read. + * nap->dynamic_load_mutex should be held. + */ +static void RosettaFlushInstructionCache(struct NaClApp *nap, + uintptr_t executable_addr, + size_t size) { +#if NACL_OSX + char *start; + char *end; + + if (!nap->in_emulator) { + return; + } + + start = (char *) (executable_addr & ~(nap->page_size - 1)); + end = (char *) NaClRoundPage(executable_addr + size, nap->page_size); + + if (0 != mprotect(start, end - start, PROT_READ) || + 0 != mprotect(start, end - start, PROT_READ | PROT_EXEC)) { + NaClLog(LOG_FATAL, "Failed to toggle PROT_EXEC: errno %d\n", errno); + } +#else + NACL_UNUSED_PARAMETER(nap); + NACL_UNUSED_PARAMETER(executable_addr); + NACL_UNUSED_PARAMETER(size); +#endif +} + static int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, uint8_t *exec_addr, uint8_t *write_addr, uint8_t *replacement_addr, @@ -111,10 +145,14 @@ static int NaClCopyCode(struct NaClApp *nap, uintptr_t guest_addr, * safe halt instructions. It is only necessary to ensure that * untrusted code runs correctly when it tries to execute the * dynamically-loaded code. + * + * For Rosetta there's no thread syncing in this one so other threads + * executing code in the same page could crash upon toggling PROT_EXEC. */ NaClFlushCacheForDoublyMappedCode(write_addr, exec_addr, size); + RosettaFlushInstructionCache(nap, (uintptr_t) exec_addr, size); return status; } @@ -776,6 +814,7 @@ int32_t NaClTextDyncodeCreate(struct NaClApp *nap, * dynamically-loaded code. */ NaClFlushCacheForDoublyMappedCode(mapped_addr, (uint8_t *) dest_addr, size); + RosettaFlushInstructionCache(nap, dest_addr, size); retval = 0; @@ -1043,6 +1082,7 @@ int32_t NaClSysDyncodeDelete(struct NaClAppThread *natp, * icache. */ NaClFlushCacheForDoublyMappedCode(mapped_addr, (uint8_t *) dest_addr, size); + RosettaFlushInstructionCache(nap, dest_addr, size); NaClTextMapClearCacheIfNeeded(nap, dest, size);