From b4db690cb30e6debd595b5ae9e11ffe006a24abc Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 10 Feb 2023 13:08:44 +0100 Subject: [PATCH 1/3] Fix GH-10370: File corruption in _php_stream_copy_to_stream_ex when using copy_file_range (#10440) copy_file_range can return early without copying all the data. This is legal behaviour and worked properly, unless the mmap fallback was used. The mmap fallback would read too much data into the destination, corrupting the destination file. Furthermore, if the mmap fallback would fail and have to fallback to the regular file copying mechanism, a similar issue would occur because both maxlen and haveread are modified. Furthermore, there was a mmap-resource in one of the failure paths of the mmap fallback code. This patch fixes these issues. This also adds regression tests using the new copy_file_range early-return simulation added in the previous commit. --- ext/zend_test/php_test.h | 1 + ext/zend_test/test.c | 17 +++++++++++++ ext/zend_test/tests/gh10370.tar | Bin 0 -> 11776 bytes ext/zend_test/tests/gh10370_1.phpt | 29 ++++++++++++++++++++++ ext/zend_test/tests/gh10370_2.phpt | 30 +++++++++++++++++++++++ ext/zend_test/tests/gh10370_3.phpt | 36 +++++++++++++++++++++++++++ ext/zend_test/tests/gh10370_4.phpt | 38 +++++++++++++++++++++++++++++ main/streams/streams.c | 23 ++++++++++++++--- 8 files changed, 170 insertions(+), 4 deletions(-) create mode 100644 ext/zend_test/tests/gh10370.tar create mode 100644 ext/zend_test/tests/gh10370_1.phpt create mode 100644 ext/zend_test/tests/gh10370_2.phpt create mode 100644 ext/zend_test/tests/gh10370_3.phpt create mode 100644 ext/zend_test/tests/gh10370_4.phpt diff --git a/ext/zend_test/php_test.h b/ext/zend_test/php_test.h index a5b11a6041d..87412ba34d3 100644 --- a/ext/zend_test/php_test.h +++ b/ext/zend_test/php_test.h @@ -53,6 +53,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test) int replace_zend_execute_ex; int register_passes; bool print_stderr_mshutdown; + zend_long limit_copy_file_range; zend_test_fiber *active_fiber; zend_long quantity_value; zend_string *str_test; diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 96898d2d71f..c6a9c7fb60a 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -650,6 +650,9 @@ PHP_INI_BEGIN() STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_BOOLEAN("zend_test.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals) +#ifdef HAVE_COPY_FILE_RANGE + STD_PHP_INI_ENTRY("zend_test.limit_copy_file_range", "-1", PHP_INI_ALL, OnUpdateLong, limit_copy_file_range, zend_zend_test_globals, zend_test_globals) +#endif STD_PHP_INI_ENTRY("zend_test.quantity_value", "0", PHP_INI_ALL, OnUpdateLong, quantity_value, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_ENTRY("zend_test.str_test", "", PHP_INI_ALL, OnUpdateStr, str_test, zend_zend_test_globals, zend_test_globals) STD_PHP_INI_ENTRY("zend_test.not_empty_str_test", "val", PHP_INI_ALL, OnUpdateStrNotEmpty, not_empty_str_test, zend_zend_test_globals, zend_test_globals) @@ -930,3 +933,17 @@ PHP_ZEND_TEST_API void bug_gh9090_void_int_char_var(int i, char *fmt, ...) { va_end(args); } + +#ifdef HAVE_COPY_FILE_RANGE +/** + * This function allows us to simulate early return of copy_file_range by setting the limit_copy_file_range ini setting. + */ +PHP_ZEND_TEST_API ssize_t copy_file_range(int fd_in, off64_t *off_in, int fd_out, off64_t *off_out, size_t len, unsigned int flags) +{ + ssize_t (*original_copy_file_range)(int, off64_t *, int, off64_t *, size_t, unsigned int) = dlsym(RTLD_NEXT, "copy_file_range"); + if (ZT_G(limit_copy_file_range) >= Z_L(0)) { + len = ZT_G(limit_copy_file_range); + } + return original_copy_file_range(fd_in, off_in, fd_out, off_out, len, flags); +} +#endif diff --git a/ext/zend_test/tests/gh10370.tar b/ext/zend_test/tests/gh10370.tar new file mode 100644 index 0000000000000000000000000000000000000000..4dbb754430d5d896c2189f882a2558470c38f851 GIT binary patch literal 11776 zcmeHtQ*17P^JRH!+qP}nwvDfDznZspZ*AMQZS&Uct!=ma+ibGgWFPivH~a9PhndMt za&pdjoQ#`=tDB{bodwAM#b9P;=459l24QAq;o@NaU+aH1%&hGHNyIGdY@8gdEUfG- z9K_5lY%I(iAjHi77dib;XrQZ`iOYYeIoMd(x&F`T|K9wc_WxbVe{}o@>3`J!9pV4W z37@%=YPCx^-A#x{2M@*F?dNxcZm8g}TH+oMCE*o=mZ&UNyWX^WkMuB6D+6-u>M)=(zuw3}kbd?IZYI@*9nchC5K0P-F>?5#bXUn5lA7VKxt{a@G z+Ql{8p|i!VgiLO`;kPiUv)|WqR&_VSA{W}tS0?m92UtIW;y&LyfQ3?5|L-Y27%cwM zh8N|WbS$vt56$=4o9%Xu`dzR6!}M&FJ9#dKl>f-WsZT3|0P#lClSXh<_AgZ zm<4*Beu0D%lc$3^6;zkM5_Zij4@FCBn)#;c%6>;N;rXYy%#qBu>%`nw%BWQR@u^;3 zaIPD{&2YVWril1lV|_*SBt8WPD*U<2wr+|6JAA$=%W9OlV+HQw zxg;sUM7h(1-dtbwQ-aRl)(F8H!B@p#<~CSvxHFHI*GT)V(+vw1wIE5EF}DV0nbw1 z!sTxEWKID(Mm(CgKaPmkpcfM@`nJFoLDm_E?<-Da->joaQ_xI!bmPY0{_CLTO@joC zcp(N9yQ~24CP@^U@pYnSc+)ZsYDp+Ddm_pcNt47b7NkD5)p9Qc5)9iNy{n*4 z;ff~1ksdQ=Tv(p^c~%1@WR&qE!Yz_zj~1-_MXYNE@~h3B_0j&;4*6=mcjGYl`4O7V z(+)_sL;K;-ta({UOIP0a8an2VkZrxdmOSvA{b9NiBYnu_?+J(W|smqYa9CYHwUa zAc)t{enM0?s3iWvF}X^n>}3Y0?-ksu#1v)lXAYO{!bEy76l73WsC1s4Ef*3OK1Y}1 z`nXR0u^V3H&aK;WHw>ZAmfDIfr2e-?OG~UjMeB~k7N3VDSatg9uPjcTEuYW&D+8># zQd4}u15Cu}M@<(5l=?TmQehXO4;kdDPbFuPA65S=h8v2NVzxk)!1-^FY%S*8)I2b? zrr>sc+$b2GK~ET24xme9=llyuRijV0N6BzW{~~hf$!}+`a;O&2iSPJz*6cK-N?}m{ zEkWIS%j64Tt}_tEfAZQWEsMmil872HYB89g@cPYpYPBA6qd|k&3KoY4c!qrpCMej< zM1;%qXPjE}=-OmQ``TaqouuzOp`)=gc$ zu`cR9{u9n_5Gl84Z64DCZw7KIL&vzf(yFAcPqvE=l|F%O@Y1J1i7I>!16A)^x!6UH z4h)!`#+eJE`5iE(Ed>8gPi}v`;+oUCM(GK;=(qQrMf`l+N6271)#Hh~v+A7jM4|jh zGHRN4ek#9f_OF@1IHXf#`zhahb?Of^1Nznrz8?pI7mj6LH+NlSMCk%+Q?phYS340` z*rMI87Lq%}@?&=7UzTbe)IVUoj^@~sgF8`93dk7I@CDd;Gg=#5emO#n%433#EJ+e^ zzuDXqRoohu?rs>+%bl6eB;D?XM(Y72J21>NQ!poj*>d9tWi{ilpFJ&DP1lnTFUH$` zP$Z3LanD_?_0fBWXV8c=Zp0~QvFie*Y*sCeSc)OX`+&&&Qoe2u9~IVJs1!mf$Qj=f~cM-#bRVf|``tCtUOn61F#vjkwV8S1qPke8zX^cF?Wv zC$~Z;>_*+KM(=gP%80DMb+IAwX$L!F#p*S-D-~Fnc=?J z&tKeKwG(!G;UX#hfx^(3n;U)~SU@rY*PlK-LW!DDYYHpt9uw7Jb!s{t!BB33d-S+K z?ETPbd>RlX2W#;)HhW=18Bc!d?0oY2HYP)G9;P(CURE8pn0i^|YVd6Z$y7;%y=|qK zmd_ZDX?-4mE42$a??zuj{zrvG=v4sMxyEhF<(-%mawEMRkM>?XxD0?N>-oj2{;?r9 z#k`>VJZg$F4YE|N`>#9m-tYz`MfzzRWpGBGas#FifD;-)d1XML@mbh{X#UGv%uOBr zLw!+(j?1`{J;XQrVtMVbB0bi(D&j$nn|{jz{>99L49ndhIwAE;DK-%*3xQQq7?A$s zQ3N>o>aAP-Dw4vYG`pwUzP<0q(d~Ab5Oe-q+tJBze`9FvlpP>FXQTF>-F^E z;e;z^gkWNQGW-X~wlbA1?6}R_{((@OB?s4A&uws~Sy(Cm_c1R;Z73J*gb-*5mmn^~ zfX2X%R&;?YYg6*dWy;A|0=KtJ!R~g$_)8~LlN1IU=@}H$`NMt663`Lmi6SY$Q;m%B zsXPPhXOBOAv&6u?3F%0`##_RC>|Umb>VW(V1(aKE`J&E6$VZIAmcMRs=@XlAv>;-$ zS!ZhFGA74|6(}a}c+1mu0pn$ENZ}38p@PsWRzj>+6SKyK-}Kum@V$tUJWW9k;C!cT zaUC08mw7=lJ^h4k{$8#0QvM|QO4YYCuYjgNTD&GS7;|Mcve{M7*TVEdB8bQsO+Ilu z5eKZHnbDuQ#5Mif0F?k&n9{1~h9@Bn)$IFCct`WB0)O(@sc^m_yxCdMV5h`SfKYM= zqAJ|h6tX!xjpH=hDot4Bvi?(OeJ5|5WX@GO zo~@p0*4u=;9!mp%19Xc}Z3q({38eGaJ(xEKU%My4Tr;U>SAIZ7qEH3B>YVKTAj}$s z&B79HImiib*$#-UT5+Jk3N5Fbalb3T86H{EIQ7YX%HWv>+jLrNdYlnii1kNC;x@0v z@B9?W52HY#ZLYY2f>z_wesfW!=qC7{1R2*uZ&OI99yF$e>=8l_E|+%WlVg z;7x~#7*X*VLe|VKJSihuwZMb0w%9A&bE2h7PO^^Tx%p0@UR?YFNuQJy+9eMl@6ux zX<}nyPP{k1wnqYs5EM-m{dPnYF|31FQvp zL}Fk5Fr0-%j%m+tVr`-Kf^S%_X{fRV$>!7M?5F+1Dn)G!ou-BCx0=I!d{WlfTB~Rx z2Q|)2MPhBLC%zyP@2aq4vzNoE{ba0s&Tp>1a;;U8&&NfXV7+PHv@Z{}YyGEpA;5jK7^bi&fK3AcO(-g$b>9g^3QKQD2!4=rh|x1y2auc0 z4&e=Wxz#uFB+P_V>>FuDQk+*mKOPHgXCz`dEhBhI<~&MHNq@gP^@I#`Qfmka0t+k= zUv~@601-fB)wBFT;nFj9cc=bLQ*Tjv#QH?0;|hxRP2xi2c;9eyzf9*ZgN?l+GP$mM z#<4?%HZE$7A5!Ybn;DEAPc0s@58jjo!#G+6ZXCWPn;|lala0lz^Pem_^mT%1_Ti=A zIqAeXZQJc6VKn)FDw@;2>yEHArVz>rtk~YYKG_;WLX9gqd+-2_LHwWqtvsRDUTAqy z7d^yqB1vwalO)~3xjHx*Uw|vM5VZlztZMO4Q$k5lu zKriZ!epduG&cTd$OC;$kRKQ76;l)If;dMV) zh6gj+5H09=u9U%zWsgRlbJ(r(dXDhf?o1?S5LRdW&}hR5Lp=KI1VcP z==S^!IafO#diA}i5MB248t;)7I`UbL*&PV-0}29H;E2A@ag(IbFZ&Vi@sIU)@;(nK zg7RuC_4Y506q!eORWZbJWTX*g6`0KXa7vi<#L}b))LpDXKL|7`5pN4SafljVLb{a{~Xc}@(@%*8MRL&VoULkf|c&e!+k z;%k+mk@EcCXf^%#tcX|SU6_!2)?MFwh~O^AZZB*xHQ&9xB=?&zO~!WJa$AT`zfaa_ zxpaaPQV+LTu5Ft20)8(YjE)R~Qq)>tvXlNZ8(NB5JSXt+;;*V7vFTw5SIA)Sh_2STU1p;(cpO_@S#qMi+)U)D~C^pAYqpt$_^ zhnn~r;`rtH6#9xNKXix+TqGm^_PIJ{RL)=jZifE$R$M0F3BFWwAnzBulq@wyLYHaS zoM|bW5>@myTCA9lbpA?-Y{gn24`+l-MSxaT-h-zCtYlZHOuO1{oO1^Ez``l0xw1)j zV)%hQ?j?+W0_se$#KX5v=HH!RKMaGzKs>Cet6x`wWfF4zZ`s7tY`h&HW_seV0j#Yv)qM`L3AQIR#W_jUZm zZdTx2@r4}ERQ6>IWx@hwcE6Fza(Pl7Y z`o&8bBNXL zpXBYp(NE_yzrcqe=Cr_+=@-840RwDpYSFDPAwO+)6RFLn2luA1qC40E9Zbzhrc zQ@k?vVqW+N-Z}%#5Fc#yTAAbUq=pSm*rG~?z?KzY=jIdm<47 zDd7SKeb9OZ$@jUW<~5e`WQp%!;=!8HVBf+MqSmNACNAcWpd9Vn_$)_WW|YXv9WTyx zAALoxeqG|Q$1fsrEh)z8_NbL~^WUHdrB%U5mFK#NNAXFu>K-Ai8#LOL9V*b+?%68# zBf>~cceyE6qeW_Uo0?n&(1u)1ytPtK7$d(-=#}B^*KhgVV;rtWm=7jb5;zC=Yvr|> z{-UYEd^bTr#!X^wFf+;$HZ@}j6q*$LmVurkf^=;!yMFAfXBrD86KDNZOisqQA3ppz z(+1v}LU?)0arH>4vkPMB6&YhybHzPmoqYLRORf#yo9ChPnN>#B-&ToXS&DDdU(Qm7 zn1o`Lza%uG32mTCGbP~-IGdf!B>r~Vl=YwThYt? ztYDj=KLBE|gSWQ5b5pm$buRw;i}wdYk_WRY%r z(e%r)23FSFwUvZx@Lwd9C{}J79|#h{8BR>&39IAKoCbGzZY+Ok!V-M6+^zVqRiJ{> zrEV6dc^Bp|bn7Np$E_3RCkj&vuf$Fwfp?hc@55`j0e!S0OUWS*g{q#a_R# zt}4qUcDWp3Be(HrP4$`umMyObUliHJ0REqn#HfdfPk*XKqqagNZbo&7RNYWd%y9_D zLlcxfU=;icL&`t)P!Zi(!33pT7X4@UNp~B4ktV zuK~!(8k`q11dCT&rg(v&%`7)2nuq4E69{WkS1XT9%P($|I1i-`J$?f>BOz0KdX5;1 zHHYs{pc0cDlCu!f(5(coxu+b;5B#ucf5aUgvcW5`Em*KZuHqBDWuL+Q0en-Rnz+K` z(o&mJ@dv^+!O$NsE^Q0ig+WnccNxt+;c?j7GMx>~+SeN_b!qBV3)yf2m9;GhTn07K zHH;Bmcl)tcXjj%5w}ul{sL}rC7EauvqoavrmlRi&CfRyzFF;_aO-U5j(e~*zc{Ihq zl^|=#PZk^mTTSO?WjKEq-C>eM`k%do=bot^P%yEfe%iVhThDtd)1odzRR=D$h@S1H zH{U1!Jn@L2%$59&asHcqM4qy?w@Yoya+6&_?j?gAVYe7@Gp6VT0SDjZkZM$_W-(Cb zCY0m!jo1C$>E>&?b_xi^9rMtN@`7L?8e)$0Fn}xXc>-l&AI&cv@?Nd~gEY=~T+x?-km|c!^eTTsE>gzY9WLic2gAOSqJ4Y9Uvxr0x_+%WG9hsATR?Z8(;h zLALdu>-GoO%I+R=KMC;Q>0sS0@=r6WMjOcsM2B(%y_@gtg&6D>T4sB;?yM|ry0Oar z>F%F5tgUM|!HoiEc}t3c?F_oJYoeWKGiDdPFmE3}oArHexsLmh@94(FNH6$yKsPS3mhOYp2=N5}N8TxU63iqi#Fsv>1lKs% zG#`-wo+$NJ3+6Y>z%*uD=xL%ZWKmOaP>zPG)hbO)W&<-ZpN=%HibH|m?eV+qfl~ru zjMSZt`2fIUS$qNxDJ;1SJywp~3emRw{m>D$vJL;sxm*DIGs16Ooi(RHH|`KxVp z{h%e>f#K7PlUD&AOkq%!(DRt~CFre$od0q*YgX>PM& zotgelyWSN>-7GoniY6C2WAc%~TmdqYkoi(h{XD05XedC@b%04Bj>hUemzNxmLDS~@ zGnNXX!X#sM8Eqru+2EI{l`QeG0&%p#xe0>ukZL7?a%VF(%CjnI{_e}Fbs-RttKIi5W1jtmMsu6e&gjDCj2Tk6|K zbn+{r$_a`oERdoODta?>PF|!Z_CRTbtnQ=kQrNU)KeMWHPF7RMqWJmn3cJtp`K`Jq zMv8IQ5U9tf%k**KJWrQ+=6O-1T_L@?7n+^}nL3LB01VbXB0AfRlONjOQ}u6Rt0~#+ zHnt=GeT!>2z{3%@szZ)H&Js&G_fqNYbtwZo3*apLbvBdPIw=_*GojdzG2Y8`Xfy_~ zsF$~Eo{+Kq8eNR4c9E9IPWjhLY?mh5wYq_vp-wwWEQY)zMZG!DRKCM#WqN8S^6zrD zVP*F@3AsPw^VQ$+G-ujWZcGL4$vKI^gE4bO+20(D93-=iCxH%tpS5uZLhzNXW;CHV zQPngJeqEMp+*pu01`-i&QgP@Q7xtd^smb#~GHMAp?!Qp)?2SJ8{e+E(D53XjahSSd zu4Vv+oCBu0L=Zc8^!0~&(G5=ReVG|65H&rc+)|c^vr$tZ4PjemKy#^ivx4zPpvdZxMKfs0!`JTY9ddH?wHQ{$*}FBbP@3y6gqgXcFq429AAlg6pM zdQFo|VAJL~Q`_Xo@{C|cF*#RujYy>7@QK=w#>>N|Eb@W@yGS8FL(@Kv_(MY+EJ{<; zi@b9{4?pHy|1KW4pkoRR@m$|uj#P?ABJYW*JQP%Fq-mhr9I@jG4n=6h;6&5ET;vH& z%3zKBr`N%+n3fHPlE& zI49_Ab#HZ=o*0I>0op)WsgSP!aywRCU4n{Lp89VKPMsfr|pSJ>xB>({bSdVUtk_OTjK{nyCd z8#0bnt3K`LDnS6JSNthghBDuue>!7sE_}ixwbZQ1=)Qa>L7=2|5Hy70S@H)PXRCR= ztP3CEwnr0d3?oD*Y0LYMCFb$)8$Dfz34G#Xn%pW(w4eJdAj4Y zvofBWQ~R%=zSx`E#uy3Fqfo$nxwqSmX!SNaBN+p(dtyPJw0dBS7%fqDHcg;oS%j)a z`7p{oAO~Hc%JiLQ&LQ@dpY#fk52fEm@Sz{XoMt<0l5RVT`R7^TS5?-3k*N?a# zt>6HE;~4FesYC+6G%)Vdl2g5m$?>H&8EE!s;y%k!+bVTR22O;vOlkJs#q;4&g%#VeP)N5}f-NPClVj7zEw|EhIEChds15#d2 zaOyI$!)E!7@Fn(MI{-;k7~^viY`7WMc>ZDK3hO-1TxTuf zH$l7hT}Dt`^`0?`))C9$CuqI_n1R4v?~Ia^4WG{c&=1-dq0wAD2) zh8mvIxxZCnEFQ68%nJ-xj!?vas|4UOK}frji*9^>(b{C(N1Hq*&Af{dz#PQmJY%#T z%|$~q_*PI+qb)Ze=KB&Cjh`K(H-EXULE!^$Y(+z^Zku-{+#H_p(1%cLm)ooS=}DAC zYDW)hVFJ*!u|#{;9oucVUqP#c6Z?R`wXrGVxH#?{_*c{fYB(iXn_OhnLp4@&E~g=u z1eT+C?vJ+9hi?u;^5%fMI$!0bStr{*nP-J3Wu9~S*AzQFi@}9bFot@YgQ0(0;v0q1 zYhWV9hCryky!{SrCZr%_eS87*d}P58++AYJ(Oxe-5?4DOhLMdiT5@_=#elwjPD(uH zyuxDYw*K^&)!~>+e~mck@21Yg5$Gbd)B|B7Ku50p_Gs3^Hik(=NL#Bn-8xDJWI%Xt z?Gj_#sKYEtIL+R%x8d%P{uAoE|IaM22r8!Hwk`J!0-AN+e1mi3-EzH!+yar5JWfnm zrOW_%!_Kw!Ks@(K`|P^FB=$w8WX zPp~0Xqa}B7ip}duPQ%GLw2)V4#@-R!C=O*iZwTR}U70NrT}CvTKObMz%PllpSe#0l&@@+YBqXaL?M_AS7|&B)6Yjl|LnNlE zmnuVQ^1X3$@6Dc>gyt!NwfpByn73R`X~s?&Sj^)5% z5)v(6=Bx9_*~-mYB~OV7?crDsTvP@8o15!dOL2 z+Lp~b4Omuq#>ZUL9T(+khZtB8wvkcyywp6C$MV?k-=1-I>qHj7SMOgQL+67B3q$4S z7S$s}PdQ%}xWD95RZ6jPxDKGN2{Qj^M7puHu@B9b(j^1j+eX3vxoXK3v{q(bBG1zs z1!X5?z-KuIm#b8f!Dl)KY~t*`3oKlphdH9+2g#>B^?Y*g-yiR~;+4)kEK-biqWD;hoxf3-jor{exsCx-Hp~ zfk8YZTXS4n3m>nWrmNDEnn`FDC=)unTF0KzZ-`q|tPCtGZgZ=cxbjc8YMK;5XCMDx TJ|+GassD1|za02~;=q3cev%i& literal 0 HcmV?d00001 diff --git a/ext/zend_test/tests/gh10370_1.phpt b/ext/zend_test/tests/gh10370_1.phpt new file mode 100644 index 00000000000..f594c3d70ec --- /dev/null +++ b/ext/zend_test/tests/gh10370_1.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-10370: File corruption in _php_stream_copy_to_stream_ex when using copy_file_range - partial copy +--EXTENSIONS-- +zend_test +phar +--SKIPIF-- + +--INI-- +zend_test.limit_copy_file_range=3584 +--FILE-- +extractTo(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370', ['testfile'])); +var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370' . DIRECTORY_SEPARATOR . 'testfile')); +?> +--EXPECT-- +bool(true) +string(40) "a723ae4ec7eababff73ca961a771b794be6388d2" +--CLEAN-- + diff --git a/ext/zend_test/tests/gh10370_2.phpt b/ext/zend_test/tests/gh10370_2.phpt new file mode 100644 index 00000000000..6f0d9da1207 --- /dev/null +++ b/ext/zend_test/tests/gh10370_2.phpt @@ -0,0 +1,30 @@ +--TEST-- +GH-10370: File corruption in _php_stream_copy_to_stream_ex when using copy_file_range - unlimited copy +--EXTENSIONS-- +zend_test +--SKIPIF-- + +--INI-- +zend_test.limit_copy_file_range=4096 +--FILE-- + +--EXPECT-- +string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" +string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" +--CLEAN-- + diff --git a/ext/zend_test/tests/gh10370_3.phpt b/ext/zend_test/tests/gh10370_3.phpt new file mode 100644 index 00000000000..2df35774287 --- /dev/null +++ b/ext/zend_test/tests/gh10370_3.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-10370: File corruption in _php_stream_copy_to_stream_ex when using copy_file_range - partial copy using stream_copy_to_stream +--EXTENSIONS-- +zend_test +--SKIPIF-- + +--INI-- +zend_test.limit_copy_file_range=3584 +--FILE-- + +--EXPECT-- +int(10240) +string(40) "a723ae4ec7eababff73ca961a771b794be6388d2" +--CLEAN-- + diff --git a/ext/zend_test/tests/gh10370_4.phpt b/ext/zend_test/tests/gh10370_4.phpt new file mode 100644 index 00000000000..69dce59ea22 --- /dev/null +++ b/ext/zend_test/tests/gh10370_4.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-10370: File corruption in _php_stream_copy_to_stream_ex when using copy_file_range - unlimited copy using stream_copy_to_stream +--EXTENSIONS-- +zend_test +--SKIPIF-- + +--INI-- +zend_test.limit_copy_file_range=4096 +--FILE-- + +--EXPECT-- +int(11776) +string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" +string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" +--CLEAN-- + diff --git a/main/streams/streams.c b/main/streams/streams.c index 20029fc73ee..de53e483c62 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -1634,8 +1634,21 @@ PHPAPI zend_result _php_stream_copy_to_stream_ex(php_stream *src, php_stream *de char *p; do { - size_t chunk_size = (maxlen == 0 || maxlen > PHP_STREAM_MMAP_MAX) ? PHP_STREAM_MMAP_MAX : maxlen; - size_t mapped; + /* We must not modify maxlen here, because otherwise the file copy fallback below can fail */ + size_t chunk_size, must_read, mapped; + if (maxlen == 0) { + /* Unlimited read */ + must_read = chunk_size = PHP_STREAM_MMAP_MAX; + } else { + must_read = maxlen - haveread; + if (must_read >= PHP_STREAM_MMAP_MAX) { + chunk_size = PHP_STREAM_MMAP_MAX; + } else { + /* In case the length we still have to read from the file could be smaller than the file size, + * chunk_size must not get bigger the size we're trying to read. */ + chunk_size = must_read; + } + } p = php_stream_mmap_range(src, php_stream_tell(src), chunk_size, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped); @@ -1650,6 +1663,7 @@ PHPAPI zend_result _php_stream_copy_to_stream_ex(php_stream *src, php_stream *de didwrite = php_stream_write(dest, p, mapped); if (didwrite < 0) { *len = haveread; + php_stream_mmap_unmap(src); return FAILURE; } @@ -1666,9 +1680,10 @@ PHPAPI zend_result _php_stream_copy_to_stream_ex(php_stream *src, php_stream *de if (mapped < chunk_size) { return SUCCESS; } + /* If we're not reading as much as possible, so a bounded read */ if (maxlen != 0) { - maxlen -= mapped; - if (maxlen == 0) { + must_read -= mapped; + if (must_read == 0) { return SUCCESS; } } From f1694409ccf5ac14da80bc85d021915a2dd8ca1d Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Fri, 10 Feb 2023 13:11:03 +0100 Subject: [PATCH 2/3] [ci skip] NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 0da29180fc2..4fdc055c86a 100644 --- a/NEWS +++ b/NEWS @@ -568,6 +568,8 @@ PHP NEWS . Fixed bug GH-9653 (file copy between different filesystems). (David Carlier) . Fixed bug GH-9779 (stream_copy_to_stream fails if dest in append mode). (Jakub Zelenka) + . Fixed bug GH-10370 (File corruption in _php_stream_copy_to_stream_ex when + using copy_file_range). (nielsdos) - Windows: . Added preliminary support for (cross-)building for ARM64. (Yun Dou) From 10f2378584ba29ef3abeb0cdc502ad32186aad96 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Fri, 10 Feb 2023 13:31:57 +0100 Subject: [PATCH 3/3] Fix concurrent testing --- ext/zend_test/tests/gh10370_1.phpt | 8 ++++---- ext/zend_test/tests/gh10370_2.phpt | 6 +++--- ext/zend_test/tests/gh10370_3.phpt | 10 +++++----- ext/zend_test/tests/gh10370_4.phpt | 8 +++----- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/ext/zend_test/tests/gh10370_1.phpt b/ext/zend_test/tests/gh10370_1.phpt index f594c3d70ec..99a2da7547b 100644 --- a/ext/zend_test/tests/gh10370_1.phpt +++ b/ext/zend_test/tests/gh10370_1.phpt @@ -16,14 +16,14 @@ zend_test.limit_copy_file_range=3584 /* Note: the value 3584 is chosen so that the mmap in _php_stream_copy_to_stream_ex() will mmap * at an offset of a multiple of 4096, which is the standard page size in most Linux systems. */ $archive = new PharData(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370.tar'); -var_dump($archive->extractTo(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370', ['testfile'])); -var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370' . DIRECTORY_SEPARATOR . 'testfile')); +var_dump($archive->extractTo(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_001', ['testfile'])); +var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_001' . DIRECTORY_SEPARATOR . 'testfile')); ?> --EXPECT-- bool(true) string(40) "a723ae4ec7eababff73ca961a771b794be6388d2" --CLEAN-- diff --git a/ext/zend_test/tests/gh10370_2.phpt b/ext/zend_test/tests/gh10370_2.phpt index 6f0d9da1207..d78a3c395b6 100644 --- a/ext/zend_test/tests/gh10370_2.phpt +++ b/ext/zend_test/tests/gh10370_2.phpt @@ -15,16 +15,16 @@ zend_test.limit_copy_file_range=4096 /* Note: the value 4096 is chosen so that the mmap in _php_stream_copy_to_stream_ex() will mmap * at an offset of a multiple of 4096, which is the standard page size in most Linux systems. */ $input_file = fopen(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370.tar', 'r'); -file_put_contents(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_out.tar', $input_file); +file_put_contents(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_002_out.tar', $input_file); fclose($input_file); var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370.tar')); -var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_out.tar')); +var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_002_out.tar')); ?> --EXPECT-- string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" --CLEAN-- diff --git a/ext/zend_test/tests/gh10370_3.phpt b/ext/zend_test/tests/gh10370_3.phpt index 2df35774287..5701c18d50c 100644 --- a/ext/zend_test/tests/gh10370_3.phpt +++ b/ext/zend_test/tests/gh10370_3.phpt @@ -14,23 +14,23 @@ zend_test.limit_copy_file_range=3584 --EXPECT-- int(10240) string(40) "a723ae4ec7eababff73ca961a771b794be6388d2" --CLEAN-- diff --git a/ext/zend_test/tests/gh10370_4.phpt b/ext/zend_test/tests/gh10370_4.phpt index 69dce59ea22..8cdb26356f0 100644 --- a/ext/zend_test/tests/gh10370_4.phpt +++ b/ext/zend_test/tests/gh10370_4.phpt @@ -15,10 +15,8 @@ zend_test.limit_copy_file_range=4096 /* Note: the value 4096 is chosen so that the mmap in _php_stream_copy_to_stream_ex() will mmap * at an offset of a multiple of 4096, which is the standard page size in most Linux systems. */ -mkdir(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370'); - $input = fopen(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370.tar', 'r'); -$output = fopen(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_out.tar', 'w'); +$output = fopen(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_004_out.tar', 'w'); var_dump(stream_copy_to_stream($input, $output)); @@ -26,7 +24,7 @@ fclose($input); fclose($output); var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370.tar')); -var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_out.tar')); +var_dump(sha1_file(__DIR__ . DIRECTORY_SEPARATOR . 'gh10370_004_out.tar')); ?> --EXPECT-- int(11776) @@ -34,5 +32,5 @@ string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" string(40) "edcad8cd6c276f5e318c826ad77a5604d6a6e93d" --CLEAN--