LWN: Comments on "Glibc change exposing bugs"

Glibc change exposing bugs

nix — Wed, 23 Oct 2013 18:09:03 +0000

What? Surely not...

... bloody hell, it does. Or many of the assembler versions do anyway. Or, rather, it assumes that distinct addresses cannot alias.

I suppose this is probably safe in practice, because if you *do* use mmap() to set up aliased regions at distinct addresses you are suddenly in hardware-dependent land (due to machines with VIPT caches such as, IIRC, MIPS, not being able to detect such aliasing at the caching level, so you suddenly need to introduce -- necessarily hardware-dependent -- cache flushes and memory barriers) so you have to know what you're doing anyway, and little things like memmove() falling back to memcpy() at unexpected times are things you're supposed to know about.

I hope.

Glibc change exposing bugs

jzbiciak — Wed, 23 Oct 2013 16:47:36 +0000

From my perspective, the two of you are in violent agreement. :-)

Glibc change exposing bugs

khim — Wed, 23 Oct 2013 15:47:06 +0000

My point was that real-world GLibC-implemented memmove does not actually work when used on POSIX system. It compares pointers and assumes that if they are different then underlying memory is also different!

Which means, strictly speaking, that memmove in GLibC is not standards-compliant :-)

Glibc change exposing bugs

nix — Wed, 23 Oct 2013 14:23:09 +0000

You appear to have read what I said exactly backwards. Of *course* C on Unix conforms to the guarantee that pointers that compare equal will point to the same object! What you can do with mmap() is produce two pointers that do *not* compare equal but which nevertheless point to the same object. This is exactly what torpedoes a fast auto-reducing-to-memcpy() memmove() implementation, since there is no way to efficiently tell if two pointers point into the same aliased region: even modifying the region via one pointer and probing via the other won't work because they could be pointing at different parts of the aliased region rather than e.g. at the start of it (you are not restricted to call memcpy()/memmove() on pointers returned from malloc(): you can copy parts of objects, and the like).

This behaviour is explicitly permitted by the Standard: segmented architectures like MS-DOS were like this decades ago. The guarantee that a == b returns nonzero only when a and b are pointers to the same object holds nonetheless. It's just a less useful guarantee than we might like.

Glibc change exposing bugs

khim — Mon, 21 Oct 2013 20:49:09 +0000

This is unfortunately the opposite of the guarantee that memmove() needs if it is to transform itself into a memmove() when needed, so (in the absence of a Standard-blessed way to normalize pointers) you are indeed forced to do a double-copy at all times when writing memmove() in Standard C.

Note that in real world there are no such guarantee (hint, hint) thus GLibC's memmove sometimes works and sometimes does not work.

Glibc change exposing bugs

nix — Mon, 21 Oct 2013 20:37:50 +0000

Your comment was interesting anyway. This is the relevant guarantee from C89 (C99 and C11 have similar wording):

If two pointers to object or incomplete types compare equal, they point to the same object. If two pointers to functions compare equal, they point to the same function. If two pointers point to the same object or function, they compare equal. If one of the operands is a pointer to an object or incomplete type and the other has type pointer to a qualified or unqualified version of void , the pointer to an object or incomplete type is converted to the type of the other operand.

The problem here is that this does not guarantee that two pointers to the same object always compare equal, but rather that if they compare equal, they are pointers to the same object (and similarly for comparison operators). We can tell if two pointers definitely are pointers within the same object, but if the comparison fails we cannot conclude anything. This is unfortunately the opposite of the guarantee that memmove() needs if it is to transform itself into a memmove() when needed, so (in the absence of a Standard-blessed way to normalize pointers) you are indeed forced to do a double-copy at all times when writing memmove() in Standard C.

Glibc change exposing bugs

jzbiciak — Fri, 18 Oct 2013 14:08:11 +0000

Yeah, I was up late and followed a link into the ancient thread. The next morning, I resumed reading, forgetting I was in a 3 year old thread. Ah well. :-)

Glibc change exposing bugs

meuh — Fri, 18 Oct 2013 13:31:16 +0000

... or a time machine to go back in 2010 ...

If we were on "stackoverflow", you would have earned the "Necromancer" badge ;)

Glibc change exposing bugs

jzbiciak — Thu, 17 Oct 2013 12:49:20 +0000

You're calling memmove verbose as compared to memcpy? Even Ken Thompson said if he had it to do over, he'd spell creat() with the final 'e'.

Glibc change exposing bugs

jzbiciak — Thu, 17 Oct 2013 12:44:16 +0000

...rather far from a Platonic ideal...

Need. More. Coffee.

Glibc change exposing bugs

jzbiciak — Thu, 17 Oct 2013 12:42:12 +0000

One major reason the remaining distinction between memcpy and memmove exists in the standard seems to be this:

To write memmove completely within conformant C, you need a malloc and a double-copy. That's because in that mythical Platonic ideal of a language, you cannot compare two pointers that do not point into the same object, and you are not guaranteed that the arguments to memmove point within the same object. That is, a fully compliant memmove would look something like this:

    void *memmove(void *dst, const void *src, size_t len)
    {
        char *srcc = (char *)src;
        char *dstc = (char *)dst;
        char *temp = malloc(len);
        size_t i;

        /* What if 'malloc' fails?  call abort()?  Unspecified! */

        for (i = 0; i != len; i++)
            temp[i] = srcc[i];

        for (i = 0; i != len; i++)
            dstc[i] = temp[i];

        free(temp);
        return dst;
    }

And, on 16-bit segmented computers or other computers lacking flat memory spaces, both of which are rather from a Platonic ideal, comparing two pointers isn't always as straightforward as you might like. So practically, memcpy offers some noticeable performance benefits on those machines.

Yes, I'm aware that the actual language in the standard says 'as if' the source was first copied to a temporary array. But, as I recall, a fully conformant C program has no other option. The 'as-if' clause allows library writers to avoid such shenanigans, without requiring them to do so. So much hair-splitting...

If it weren't for that, you could make the argument that separate memcpy and memmove were historical accidents, and change the C standard at some point to remove the restrictions on memcpy to make them both equivalent. That new memcpy would then adhere to Rusty's Maxim, or at least come much closer. And, from the thread linked above, that's pretty much what BSD did, it sounds like.

As a half step, you could define memcpy as always copying forward, to make "sliding down" safe, but that just seems a little goofy for a number of reasons.

I'm personally with Linus that the glibc breakage seems gratuitous. I'd lean towards making memcpy and memmove equivalent if their performance is largely indistinguishable. Arguing that the software is broken when it worked for year with the old library reminds me of this silly meme. It's the kind of hair-splitting that only a bureaucrat or chapter-verser could love.

Glibc change exposing bugs

paulj — Sat, 27 Nov 2010 13:26:34 +0000

Well, if you want a web interface style GUI for info, but don't want to install either of the main two GUI environments, then... ;) Pinfo possibly is closest to what you want. A lynx/elinks style browser interface, for the terminal.

Glibc change exposing bugs

SEJeff — Fri, 26 Nov 2010 13:37:43 +0000

cvs for a filesystem? wow

Glibc change exposing bugs

Spudd86 — Fri, 26 Nov 2010 01:33:33 +0000

Don't use KDE either... I use XFCE and try to keep most of the GNOME stuff not installed.

Glibc change exposing bugs

sfeam — Fri, 26 Nov 2010 00:40:54 +0000

You could use konqueror instead
konqueror info:tar

Glibc change exposing bugs

Spudd86 — Fri, 26 Nov 2010 00:31:13 +0000

Don't use GNOME, I wonder how much of GNOME Yelp pulls in

Glibc change exposing bugs

paulj — Thu, 25 Nov 2010 22:13:17 +0000

Have you tried going to System -> Help? GNOME's "Yelp" supports browsing info docs - providing a web browser style GUI...

Kernel 2.6.36 broke my CentOS-5 Gnome 2.16 battery info

dag- — Thu, 25 Nov 2010 18:10:41 +0000

I intended to say that ELRepo packages were built specifically on RHEL 5, (but are intended the various RHEL rebuilds as well). In the case of my laptop, it was using CentOS-5 but in the meantime moved to RHEL 6 instead...

Kernel 2.6.36 broke my CentOS-5 Gnome 2.16 battery info

dag- — Thu, 25 Nov 2010 17:32:51 +0000

> The rule is simply "we don't break user-space".

Well, I don't know how general that rule is, because kernel 2.6.36 ripped out an important set of /proc/acpi entries that are still used on older Gnome releases (eg. CentOS-5).

A separate project, named ELRepo, provides backported kernel modules, but also the current mainline kernel built specifically for CentOS-5. Which is great for testing/running the latest kernel with a stable and trusted distribution. Since 2.6.36, not anymore, as my laptop couldn't provide proper ACPI information, and as such couldn't suspend/hibernate before running out of power :-(

More information about this, and other breakage is available from:

http://elrepo.org/tiki/kernel-ml

Glibc change exposing bugs

Spudd86 — Thu, 25 Nov 2010 16:39:58 +0000

Well yea, but Lennart Pottering does have a blog post where he says exactly what subset of ALSA's API you should restrict yourself to, perhaps someone should put that into the ALSA docs.

Glibc change exposing bugs

foom — Thu, 25 Nov 2010 16:07:57 +0000

If the documentation wasn't so terrible, this probably wouldn't be a problem. It doesn't give *any* clue that, for example, a developer shouldn't use the mmap functions. In fact it makes it sound like you should use them, because they're zero-copy (and that's better, right?)

Glibc change exposing bugs

Spudd86 — Thu, 25 Nov 2010 15:22:07 +0000

info's major problem is that it's interface SUCKS, there's no real 'back' command, the keybindings are just plain weird (unless you're an EMACS user...).

It'd be nice to have an info viewer that converts to HTML on the fly and uses webkit to render it.

Glibc change exposing bugs

Spudd86 — Thu, 25 Nov 2010 15:13:21 +0000

It's not so much that ALSA is easy to misuse (although it probably is), it's that certain parts of it are impossible to emulate from userspace. An app that actually NEEDS those parts is NOT misusing the API when it uses them (for example, pulseaudio itself uses those bits).

The problem is that most apps don't actually need the those bits, so they just needlessly break software like pulseaudio (and also break on bluetooth audio too).

Pulseaudio does use those unemulatable APIs, but it also falls back if they don't work, and it has good reasons to use those APIs (so it can hand over large chunks of audio data, but still be able to decide it wants to change that same data later (if for example something else starts playing audio), this saves you power because pulse won't wake your CPU as much, but it also uses APIs that don't emulate well AND until pulse came along nobody ever tried to do that sort of thing so it broke)

The real problem here

linuxrocks123 — Fri, 19 Nov 2010 02:14:30 +0000

Well, actually, in this particular case you could probably rewrite the binary to call memmove instead of memcpy fairly easily if you really wanted to.

---linuxrocks123

Glibc change exposing bugs

oak — Wed, 17 Nov 2010 19:08:13 +0000

> And one should know that GCC provides inline versions of such functions

Wasn't this article about Glibc memcpy(), not the GCC (libgcc?) one?

Anyway, AFAIK GCC does that only if code is compiled with optimizations. Valgrind and -O0 compiled code are speed-wise pretty horrible combination though. Then it might be better to use one of the other memory debugging tools that don't do CPU emulation like Valgrind does...

Note that GCC doesn't inline its memcpy() code just for explicit (fixed size) memcpy() calls. Inlined version may also be used for assignments and developers are able to mess up addresses of variables used in thing like this too:

  struct foobar_t *a = arg1, *b = arg2;
  ...
  *a = *b;

(I found this issue on implicit GCC memcpy() when my code didn't have correct alignment for one of above kind of pointers on platform that required things to be properly aligned. It triggering a kernel alignment exception handler bug had me scratching my head until more knowledgeable colleague came to rescue... I think with overlapping pointer addresses results may be even more mysterious as they show up later.)

This isn't either/or. Phase in such changes instead!

xilun — Wed, 17 Nov 2010 16:23:09 +0000

> There is no way to be sure that this change is not quietly damaging untold amounts of data without auditing every use of memcpy everywhere to ensure that it is doing the right thing.

There is also no way to be sure that this change is not _fixing_ untold amounts of data corruption when the memcpy is done backward without auditing every use of memcpy :)
Anyway, C being what it is, this is a little ridiculous to do a fixation on that particular change, because some other changes exposing bugs are done every day, hundred at a time. So really, you have no way after ANY upgrade to be sure that memory corruption won't mysteriously happens when they previously did not. If that's a problem for you, don't ever update anything => problem magically solved.

> given that Glibc has symbol versioning, maybe they should use it?

Nope. Symbol versioning is for ABI changes, and symbol versioning does not even pretend to automatically solve every problem ABI changes has been shown to cause. The memcpy implementation change is not even an ABI change.

> Your last sentence sums up the problem: "Hopefully legitimate uses are not affected". I think we should expect stronger guarantees from glibc than "hopefully".

There was a problem only in the sentence. The "hopefully" is not needed. Legitimate users of memcpy will not be affected.

This isn't either/or. Phase in such changes instead!

meuh — Wed, 17 Nov 2010 16:09:29 +0000

Hopefully, every programmer run their programs under valgrind once in a while.

This isn't either/or. Phase in such changes instead!

mrshiny — Wed, 17 Nov 2010 15:52:37 +0000

Yes, it is a bug. Sure, the application is responsible for using APIs properly. But here we have a situation where a library has worked one way for years, and then suddenly works a different way. There was no way for the apps in question to detect the bugs because the code worked perfectly before. Now, due to a library upgrade, those apps don't work. In some cases there is data corruption. The corruption might happen silently. There is no way to be sure that this change is not quietly damaging untold amounts of data without auditing every use of memcpy everywhere to ensure that it is doing the right thing.

And this means that not only do you have to fix all source code which is wrong and issue new binaries, but you shouldn't upgrade to this version of Glibc because you might have an app somewhere that wasn't fixed, or isn't fixed in the version you have installed.

Glibc is a critical library in the system. Almost every program uses it. As such, it is their responsibility to treat ABI changes very carefully. Sure, this is not a change in the specification, it is an unintended consequence and it's due to those stupid lazy programmers who didn't read the spec or didn't care or whatever. Or inadvertently introduced errors when their code was changed. Or changed something without realizing that this change would result, somewhere, in a call to overlapping memcpy. Given that the bug was hard to identify (at least for some cases), and given that Glibc has symbol versioning, maybe they should use it?

Your last sentence sums up the problem: "Hopefully legitimate uses are not affected". I think we should expect stronger guarantees from glibc than "hopefully".

This isn't either/or. Phase in such changes instead!

meuh — Wed, 17 Nov 2010 15:14:43 +0000

Remember: this bug exists for all users of glibc even if they compiled their apps a long time ago but recently updated glibc.

It's not a bug. And it doesn't affect all users.

Hopefully, legitimate uses (regarding to specification) of memcpy() are not affected by the optimisation in newer glibc.

Glibc change exposing bugs

meuh — Wed, 17 Nov 2010 15:02:48 +0000

But once inlined, those functions can't be overloaded with a LD_PRELOAD module nor with a tool like valgrind. And overlap will happen in user's back.

Glibc change exposing bugs

meuh — Wed, 17 Nov 2010 14:45:37 +0000

As I said here https://bugzilla.redhat.com/show_bug.cgi?id=638477#c116

Using -D_FORTIFY_SOURCE enable only check for overflow when source and destination length are known (or can be computed).

_chk() variant of memset(), memcpy(), etc. didn't check for overlap.

And one should know that GCC provides inline versions of such functions, so valgrind won't be able to overload them and provide stronger argument checking.

Glibc change exposing bugs - a bug in proposed memcpy

promotion-account — Tue, 16 Nov 2010 16:45:16 +0000

Look at the one proposed by Linus:
void *memcpy(void *dst, const void *src, size_t size)
{
void *orig = dst;
asm volatile("rep ; movsq"
:"=D" (dst), "=S" (src)
:"0" (dst), "1" (src), "c" (size >> 3)
:"memory");
asm volatile("rep ; movsb"
:"=D" (dst), "=S" (src)
:"0" (dst), "1" (src), "c" (size & 7)
:"memory");
return orig;
}

For completeness, this should have an "rcx" clobber, or GCC may believe that this important register will not change after each assembly snippet. Such a bug may get triggered if GCC aggressively inlined the code, which occurs in a good number of cases given its optimizer competency.

--Darwish

Glibc change exposing bugs

renox — Mon, 15 Nov 2010 16:43:09 +0000

Too late!

And memcpy should also be named as mem_unsafe_copy, but yes if you tell developers to use safe function by default and to optimize only when they can show benchmarks that the optimisation will make a difference, then yes, you'd get probably better software (if a bit slower).

Glibc change exposing bugs

slashdot — Mon, 15 Nov 2010 12:15:59 +0000

That rationale seems a bit dubious.

In particular, won't just doing all reads before all writes ensure no aliasing regardless of CPU operation?
I think there are enough callee-clobbered registers on x86-64 to allow that.

That is, do this:
movq (%rsi), %rax
movq 8(%rsi), %rdx
movq %rax, (%rdi)
movq %rdx, 8(%rdi)

Also, their backward copy obviously aliases if rsi is 0xf00c instead of 0xf004. I'm not sure why either of these cases should be intrinsically more frequent.

Glibc change exposing bugs

cladisch — Mon, 15 Nov 2010 11:50:24 +0000

Backward copying avoids cache address aliasing effects on these processors:
http://lists.freedesktop.org/archives/pixman/2010-August/...

Glibc change exposing bugs

nix — Mon, 15 Nov 2010 10:39:15 +0000

Ah. I interpreted it as 'account bought to promote something else', and got confused because most advertisers would try to lie about it and *not* mention their affiliations :)

'Promotion' is a word with many meanings...

Glibc change exposing bugs

nix — Mon, 15 Nov 2010 10:38:12 +0000

Yes, they are the most useful documentation we have, especially for things that do not have glibc wrappers. But even for the kernel they are descriptive, and for things for which the glibc wrappers are the primary implementation (like readdir()) or for which there is no kernel component, the manpages are completely after-the-fact. (As far as I can tell the glibc project no longer bothers to document anything at all. There are lots of utterly undocumented things in glibc's allegedly public interface.)

Glibc change exposing bugs

mpr22 — Mon, 15 Nov 2010 09:47:05 +0000

I must confess to being utterly boggled by the notion of a backwards block copy (decrementing address) being faster than the forward (incrementing address) version. I mean, doesn't backward copying break the memory controller prefetch?

Glibc change exposing bugs

dlang — Mon, 15 Nov 2010 08:13:38 +0000

you are assuming that the program authors care about Irix, AIX, Solaris, or anything else.

most programs do not start off being written portably, usually portability is something that shows up after the program starts being used when people ask about using it on other platforms (and it's not uncommon for it to wait until those people asking submit patches)

not saying that this is right, just saying that it's the way things are. When Solaris dominated the same thing happened favoring it.

Glibc change exposing bugs

promotion-account — Mon, 15 Nov 2010 01:37:49 +0000

(btw, your account name is... *interesting*.)

That's descriptive anonymity :)

Readers usually give higher weight to subscribers opinions here, so this handle honestly states that I'm a promoted guest.