Date: Sun, 10 Sep 2023 09:53:03 -0600 From: Warner Losh <imp@bsdimp.com> To: Robert Clausecker <fuz@fuz.su> Cc: FreeBSD Current <current@freebsd.org> Subject: Re: sed in CURRENT fails in textproc/jq Message-ID: <CANCZdfoCBXxNzfTQ03bHwgOkRMoZAAkj_yxZ58i82e0chMWEog@mail.gmail.com> In-Reply-To: <ZP3GWo3ZXahC8_2_@fuz.su> References: <30a59ac0-28fd-f6ed-505c-9ef6d8e84fc3@tsoft.com> <CAALwa8kAaDw80h2cbaeLOmw2G9EpRo6dJFZcFFc99mAU_7xkRA@mail.gmail.com> <86edj7qnia.fsf@ltc.des.no> <ZP1LPZcC99zIS1Jp@fuz.su> <CANCZdfpesLu%2B5YLB4-irR7XSSSM1wXPLH58TbefPzvGo=q1xgg@mail.gmail.com> <ZP3GWo3ZXahC8_2_@fuz.su>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000c1cb950605033405 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Sep 10, 2023, 7:36 AM Robert Clausecker <fuz@fuz.su> wrote: > Hi Warner, > > I have pushed a fix. It should hopefully address those failing tests. > The same issue should also affect memcmp(), but unlike for memchr(), it i= s > illegal to pass a length to memcmp() that extends past the actual end of > the buffer as memcmp() is permitted to examine the whole buffer regardles= s > of where the first mismatch is. > > I am considering a change to improve the behaviour of memcmp() on such > errorneous inputs. There are two options: (a) I could change memcmp() th= e > same way I fixed memchr() and have implausible buffer lengths behave as i= f > the buffer goes to the end of the address space or (b) I could change > memcmp() to crash loudly if it detects such a case. I could also > (c) leave memcmp() as is. Which of these three choices is preferable? > What does the standard say? I'm highly skeptical that these corner cases ar= e UB behavior. I'd like actual support for this statement, rather than your conjecture that it's illegal. Even if you can come up with that, preserving the old behavior is my first choice. Especially since many of these functions aren't well defined by a standard, but are extensions. As for memchr, https://pubs.opengroup.org/onlinepubs/009696799/functions/memchr.html has no such permission to examine 'the entire buffer at once' nor any restirction as to the length extending beyond the address space. I'm skeptical of your reading that it allows one to examine all of [b, b + len), so please explain where the standard supports reading past the first occurance. Also, all the tests that started failing with your commit, available here, https://ci.freebsd.org/job/FreeBSD-main-amd64-test/24136/ should be fixed at a bare minimum. Yes, there's a lot of other tests that are failing: those should be fixed or disabled also. Have these tests been fixed? They are showing up as failed still in the latest run https://ci.freebsd.org/job/FreeBSD-main-amd64-test/24141/ shows them still failing by my reading. Also, we're several weeks in, and we're still fixing basic, fundamental functions that show breakage. This indicates to me that proper care hasn't been taken to replace the critical functions in the tree. It also indicates to me that a stronger level of rigor is needed given that the problems are trickling in every few days to weeks. This suggests that the unusual step of backing all this work out of stable/14 and releng/14.0 would be indicated until we can go through this process to the point that the new bugs have stopped (which given the decay rate seen so far suggests we won't be done before 14.0 is released). i know this is an enormous pain, but I think the track record so far supports backing these out of the release we're about to do. They just arrived too late for the normal 'cooking' process of FreeBSD to sufficiently cook them in time for 14.0. Warner Yours, > Robert Clausecker > > Am Sun, Sep 10, 2023 at 05:51:43AM -0600 schrieb Warner Losh: > > On Sat, Sep 9, 2023, 10:51 PM Robert Clausecker <fuz@fuz.su> wrote: > > > > > Greetings, > > > > > > I apologise for the inconvenience. The issue seems to boil down to > > > various places calling > > > > > > memchr(buf, c, SIZE_MAX); > > > > > > which causes an overflow when my newly written memchr() computes buf = + > > > len to find the end of the buffer. A patch to alleviate this issue c= an > > > be found here: > > > > > > > > > > http://fuz.su/~fuz/freebsd/0001-lib-libc-amd64-string-memchr.S-fix-behavi= our-with-ov.patch > > > > > > Please check if it does the trick for you. If yes, I'll go ahead and > > > push it tomorrow-ish. > > > > > > > > > There are half a dozen or do kyua tests that are likely failing because > of > > this or other reasons related to strings. When you push this fix you'l= l > > get the list. They are hidden among about 80 or so networking tests tha= t > > fail. I plan on disabling those tests soon If no one fixes them. > > > > Warner > > > > > > > > Yours, > > > Robert Clausecker > > > > > > Am Sat, Sep 09, 2023 at 07:12:29PM +0200 schrieb Dag-Erling Sm=C3=B8r= grav: > > > > Antoine Brodin <antoine@freebsd.org> writes: > > > > > Yuri <yuri@freebsd.org> writes: > > > > > > Either something has changed in sed(1) in CURRENT, or sed just > fails > > > > > > during the configure stage of textproc/jq: > > > > > > > > > > > > sed: No error: 0 > > > > > > checking for sys/cygwin.h... eval: ${+...}: Bad substitution > > > > > This seems to be a recent issue (less than 5 days). > > > > > Hundreds of configure scripts now fail to run on 15-current due t= o > > > > > this sed failure: [...] > > > > > > > > Try adding ARCHLEVEL=3Dscalar to CONFIGURE_ENV on one of these. If > that > > > > helps, yell at fuz@ :) > > > > > > > > DES > > > > -- > > > > Dag-Erling Sm=C3=B8rgrav - des@FreeBSD.org > > > > > > > > > > -- > > > () ascii ribbon campaign - for an 8-bit clean world > > > /\ - against html email - against proprietary attachments > > > > > > > > -- > () ascii ribbon campaign - for an 8-bit clean world > /\ - against html email - against proprietary attachments > --000000000000c1cb950605033405 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"auto"><div><br><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Sun, Sep 10, 2023, 7:36 AM Robert = Clausecker <<a href=3D"mailto:fuz@fuz.su" target=3D"_blank">fuz@fuz.su</= a>> wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 = 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Warner,<br> <br> I have pushed a fix.=C2=A0 It should hopefully address those failing tests.= <br> The same issue should also affect memcmp(), but unlike for memchr(), it is<= br> illegal to pass a length to memcmp() that extends past the actual end of<br= > the buffer as memcmp() is permitted to examine the whole buffer regardless<= br> of where the first mismatch is.<br> <br> I am considering a change to improve the behaviour of memcmp() on such<br> errorneous inputs.=C2=A0 There are two options: (a) I could change memcmp()= the<br> same way I fixed memchr() and have implausible buffer lengths behave as if<= br> the buffer goes to the end of the address space or (b) I could change<br> memcmp() to crash loudly if it detects such a case.=C2=A0 I could also<br> (c) leave memcmp() as is.=C2=A0 Which of these three choices is preferable?= <br></blockquote></div></div><div dir=3D"auto"><br></div><div dir=3D"auto">= What does the standard say? I'm highly skeptical that these corner case= s are</div><div dir=3D"auto">UB behavior.</div><div><br></div><div>I'd = like actual support for this statement, rather than your conjecture that it= 's</div><div>illegal. Even if you can come up with that, preserving the= old behavior is my</div><div>first choice. Especially since many of these = functions aren't well defined by</div><div>a standard, but are extensio= ns.</div><div><br></div><div>As for memchr, <a href=3D"https://pubs.opengro= up.org/onlinepubs/009696799/functions/memchr.html">https://pubs.opengroup.o= rg/onlinepubs/009696799/functions/memchr.html</a></div><div>has no such per= mission to examine 'the entire buffer at once' nor any restirction<= /div><div>as to the length extending beyond the address space. I'm skep= tical of your reading</div><div>that it allows one to examine all of [b, b = + len), so please explain where the standard</div><div>supports reading pas= t the first occurance.<br></div><div><br></div><div>Also, all the tests tha= t started failing with your commit, available here,<br></div><div dir=3D"au= to"><br></div><div dir=3D"auto"><a href=3D"https://ci.freebsd.org/job/FreeB= SD-main-amd64-test/24136/" target=3D"_blank">https://ci.freebsd.org/job/Fre= eBSD-main-amd64-test/24136/</a><br></div><div dir=3D"auto"><br></div><div>s= hould be fixed at a bare minimum. Yes, there's a lot of other tests</di= v><div>that are failing: those should be fixed or disabled also. Have these= </div><div>tests been fixed? They are showing up as failed still in the lat= est run</div><div><br></div><div><a href=3D"https://ci.freebsd.org/job/Free= BSD-main-amd64-test/24141/">https://ci.freebsd.org/job/FreeBSD-main-amd64-t= est/24141/</a></div><div><br></div><div>shows them still failing by my read= ing.<br></div><div><br></div><div>Also, we're several weeks in, and we&= #39;re still fixing basic, fundamental</div><div>functions that show breaka= ge. This indicates to me that proper care</div><div>hasn't been taken t= o replace the critical functions in the tree. It also</div><div>indicates t= o me that a stronger level of rigor is needed given that the</div><div>prob= lems are trickling in every few days to weeks. This suggests that</div><div= >the unusual step of backing all this work out of stable/14 and releng/14.0= </div><div>would be indicated until we can go through this process to the p= oint</div><div>that the new bugs have stopped (which given the decay rate s= een so</div><div>far suggests we won't be done before 14.0 is released)= . i know this is</div><div>an enormous pain, but I think the track record s= o far supports backing</div><div>these out of the release we're about t= o do. They just arrived too late</div><div>for the normal 'cooking'= process of FreeBSD to sufficiently cook them</div><div>in time for 14.0.<b= r></div><div><br></div><div>Warner<br></div><div dir=3D"auto"><br></div><di= v dir=3D"auto"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote"= style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Yours,<br> Robert Clausecker<br> <br> Am Sun, Sep 10, 2023 at 05:51:43AM -0600 schrieb Warner Losh:<br> > On Sat, Sep 9, 2023, 10:51 PM Robert Clausecker <<a href=3D"mailto:= fuz@fuz.su" rel=3D"noreferrer" target=3D"_blank">fuz@fuz.su</a>> wrote:<= br> > <br> > > Greetings,<br> > ><br> > > I apologise for the inconvenience.=C2=A0 The issue seems to boil = down to<br> > > various places calling<br> > ><br> > >=C2=A0 =C2=A0 =C2=A0memchr(buf, c, SIZE_MAX);<br> > ><br> > > which causes an overflow when my newly written memchr() computes = buf +<br> > > len to find the end of the buffer.=C2=A0 A patch to alleviate thi= s issue can<br> > > be found here:<br> > ><br> > ><br> > > <a href=3D"http://fuz.su/~fuz/freebsd/0001-lib-libc-amd64-string-= memchr.S-fix-behaviour-with-ov.patch" rel=3D"noreferrer noreferrer" target= =3D"_blank">http://fuz.su/~fuz/freebsd/0001-lib-libc-amd64-string-memchr.S-= fix-behaviour-with-ov.patch</a><br> > ><br> > > Please check if it does the trick for you.=C2=A0 If yes, I'll= go ahead and<br> > > push it tomorrow-ish.<br> > ><br> > <br> > <br> > There are half a dozen or do kyua tests that are likely failing becaus= e of<br> > this or other reasons related to strings.=C2=A0 When you push this fix= you'll<br> > get the list. They are hidden among about 80 or so networking tests th= at<br> > fail. I plan on disabling those tests soon If no one fixes them.<br> > <br> > Warner<br> > <br> > ><br> > > Yours,<br> > > Robert Clausecker<br> > ><br> > > Am Sat, Sep 09, 2023 at 07:12:29PM +0200 schrieb Dag-Erling Sm=C3= =B8rgrav:<br> > > > Antoine Brodin <<a href=3D"mailto:antoine@freebsd.org" re= l=3D"noreferrer" target=3D"_blank">antoine@freebsd.org</a>> writes:<br> > > > > Yuri <<a href=3D"mailto:yuri@freebsd.org" rel=3D"nor= eferrer" target=3D"_blank">yuri@freebsd.org</a>> writes:<br> > > > > > Either something has changed in sed(1) in CURRENT,= or sed just fails<br> > > > > > during the configure stage of textproc/jq:<br> > > > > ><br> > > > > > sed: No error: 0<br> > > > > > checking for sys/cygwin.h... eval: ${+...}: Bad su= bstitution<br> > > > > This seems to be a recent issue (less than 5 days).<br> > > > > Hundreds of configure scripts now fail to run on 15-cur= rent due to<br> > > > > this sed failure: [...]<br> > > ><br> > > > Try adding ARCHLEVEL=3Dscalar to CONFIGURE_ENV on one of the= se.=C2=A0 If that<br> > > > helps, yell at fuz@ :)<br> > > ><br> > > > DES<br> > > > --<br> > > > Dag-Erling Sm=C3=B8rgrav - des@FreeBSD.org<br> > > ><br> > ><br> > > --<br> > > ()=C2=A0 ascii ribbon campaign - for an 8-bit clean world<br> > > /\=C2=A0 - against html email=C2=A0 - against proprietary attachm= ents<br> > ><br> > ><br> <br> -- <br> ()=C2=A0 ascii ribbon campaign - for an 8-bit clean world <br> /\=C2=A0 - against html email=C2=A0 - against proprietary attachments<br> </blockquote></div></div></div> </div> --000000000000c1cb950605033405--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoCBXxNzfTQ03bHwgOkRMoZAAkj_yxZ58i82e0chMWEog>