Glibc change exposing bugs

Posted Nov 11, 2010 5:33 UTC (Thu) by PaulWay (guest, #45600)
Parent article: Glibc change exposing bugs

Interesting that Linus didn't patch it by simply calling memmove with the same arguments and passing back the same return.

Interesting that no-one's suggested we fix the obviously ambiguous wording in the man page. It seems that trusting the C programmer to know the difference between memcpy and memmove - whose names do _not_ imply anything about their behaviour - is a bad thing. Rusty's Hierarchy of API Design scores another victim, and yet no-one wants to fix either the API, the documentation or the behaviour.

Interesting that the question of why backwards-copying is necessary remains (AFAICS) unanswered. Has anyone actually tested whether the loop can be written with a forwards-copy and whether it performs better or worse than the backwards-copy and/or the Linus brute-force method?

Interesting that everyone who doesn't want to change memcpy to do checks or warn or everything asserts, without much actual evidence, that it would be a Bad Thing. Citation needed, or at least some crude benchmarks or numbers.

In my uninformed opinion it would be better to have one version of memcpy (memmove implies that the memory is absent from the source once completed, which is not true) that does the checks. The tiny overhead will be nothing compared to the page faults you're almost certainly incurring with repeated use. Run some tests, real world or otherwise, to see whether it really makes any difference. If it doesn't, make memmove a defined alias for memcpy, update the documentation, and everyone wins. The API remains the same, badly written applications don't die because of an underlying implementation change, lazy programmers have their arses saved, everyone wins.

But what do I know? I'm still writing the test.

Have fun,

Paul

Glibc change exposing bugs

Posted Nov 11, 2010 9:54 UTC (Thu) by mpr22 (subscriber, #60784) [Link] (2 responses)

That particular kind of lazy programmer deserves to lose, unfortunately.

Glibc change exposing bugs

Posted Nov 12, 2010 0:17 UTC (Fri) by PaulWay (guest, #45600) [Link] (1 responses)

True. I for one think the best solution to this whole debacle is for Adobe to fix their code. But we can't do anything about that, and I don't hold any hope of Adobe putting this at any higher priority than 'do it after learning Etruscan'.

I personally think we should do more tests with this kind of thing. Use the same LD_PRELOAD trick that Linus used to fix the problem to see if any other applications are assuming that memcpy will work on overlapping regions. See if we can find any other little abuses of standards, or ambiguities in them, that might catch us out in the future. And not just fix the code, but fix the standard. No-one, I hope, is saying that people should be using memcpy as if it were overlap-safe - just that existing code which does is sort of exempt from criticism.

Ah well, maybe we'll all look back on this and laugh.

Have fun,

Paul

Glibc change exposing bugs

Posted Nov 12, 2010 7:41 UTC (Fri) by hozelda (guest, #19341) [Link]

"Fixing" the standard, if you mean going back to ISO C, might not be very likely to happen.

Glibc change exposing bugs

Posted Nov 11, 2010 12:15 UTC (Thu) by alankila (guest, #47141) [Link]

You make perfect sense.

But sadly, I don't think it is possible to make people here accept that simply aliasing memcpy() to memmove() is actually the best solution. I bet the difference wouldn't show in anything but carefully constructed microbenchmarks, and yet we would be able to squash a whole class of bugs at once.

However, I do believe that the best use of increased CPU power is to spend it on simplifying the system, because that allows raising the complexity bar somewhere else higher up. (I believe in complexity budget: a finite number of things are possible. You are best off spending that complexity budget on features close to the user than on those close to the metal, so making people not have to care about difference for memmove() vs. memcpy() allows them to spend time caring about something that's far more useful.)

Glibc change exposing bugs

Posted Nov 11, 2010 17:24 UTC (Thu) by RobSeace (subscriber, #4435) [Link]

> memmove implies that the memory is absent from the source once completed,
> which is not true

Well, actually, it IS kind of true... If the regions overlap (which is the sole
point of using memmove()), then it implies that the source region will indeed
no longer contain the data it previously contained, since at least part of it
would've been overwritten by the memmove() to the (overlapping) destination
region...

I side with the glibc people: any C programmer worth a damn knows better than
to use memcpy() on overlapping regions... Anyone that does so is writing known
buggy code that will fail to work on many systems... That it just happened to
have worked by chance until now on glibc doesn't matter a bit... There are
lots of subtle bugs you can make that appear to work fine until something
changes and exposes them... You see it in buffer overflows all the time; if
you overflow just a small amount and there's some meaningless variable there
in memory that you overflow into, no harm done, no crashing, no noticable bad
behavior at all... But, compile with a different optimization level, or change
the code in a certain way, and BAM!, that variable is no longer there to catch
the overflow, and you end up trashing something important... Now, are you
seriously going to say GCC should support such obviously buggy code by making
sure to always continue laying out variables in memory just as it did the
first time, so that the overflow causes no harm? If not, then how is this
glibc change any different at all?