Re: firefox52 core dump on RPI2 NetBSD9.1

[Jeffrey Walton <noloader%gmail.com@localhost>]

On Sat, Nov 28, 2020 at 3:15 AM <tlaronde%polynum.com@localhost> wrote:
>
> On Sat, Nov 28, 2020 at 08:41:50AM +0100, Martin Husemann wrote:
> > On Fri, Nov 27, 2020 at 11:29:13PM -0500, Jeffrey Walton wrote:
> > > > Concerning the core dumps, there is another thing to look at:
> > > > _FORTIFY_SOURCE. There are checks about the use of strings functions
> > > > that can cause an abort even if the actual use is probably, with
> > > > a classic C implementation, safe---I hit it with a strcpy() that was
> > > > removing a prefix simply shifting bytes left in a buffer; it didn't
> > > > cause any harm before 9.* and now aborts because src and dst overlap.
> > >
> > > Off-topic, but that's undefined behavior. You should use memmove in
> > > the case when src and dest overlap.
> >
> > This is very important. While there are lots of undefined behaviours
> > where we *know* the concrete behaviour (for all architectures) on
> > NetBSD is OK (so we could choose to ignore the issue), this example is
> > one that breaks in subtle ways (depending on architecture and alignemnt
> > of the string buffer) with some assembler implementations of the string
> > operation.
> >
> > The "it didn't cause any harm before 9.*" is due to limited testing, small
> > selection of architectures, or just plain luck in this case.
>
> It is also caused by knowledge of C: one imagine that strcpy is
> implemented doing _string_ manipulation (because of its name) and what one
> will do programming it directly that is:
>
> p = buf;
> q = buf + positive_shift;
> while ((*p++ = *q++) != '\0')
>         ;
>
> Note: a negative shift will cause a crash if the bytes before buf
> are all != '\0' since it will be infinite.
>
> And no, it was not just "plain luck" and "limited testing" since
> it worked anywhere, not only on NetBSD, and on all arch (and it
> still works; it does not work with _FORTIFY_SOURCE=2 because of
> the check; but the code works). I'm not saying that reading the
> POSIX spec (I don't know for standard C) it is permitted: it is
> undefined; I'm not saying that I have not fixed it. But there are
> probably a lot of code out there that have this kind of usage as
> well as, for example, programming zeroing structures assuming that
> NULL is indeed a all zero address while this is not the case in
> standard C: NULL is a defined value, treated as 0 in conditionals
> by the compiler, but it is not mandatorily defined as address 0.

A really good example of problems caused by _not_ using a memmove for
overlapping buffers is at
https://bugzilla.redhat.com/show_bug.cgi?id=638477. Also see
https://lwn.net/Articles/416821/.

Jeff