Glibc change exposing bugs

Posted Nov 11, 2010 1:52 UTC (Thu) by gus3 (guest, #61103)
In reply to: Glibc change exposing bugs by jwb
Parent article: Glibc change exposing bugs

No, it is simple integer math:

if ((p1 + length <= p2) || (p2 + length <= p1)) {
crash_and_burn();
}

It is not a sophisticated test, and the more noise it makes about buggy parameters, the sooner the calling code will get fixed.

Glibc change exposing bugs

Posted Nov 11, 2010 2:05 UTC (Thu) by gus3 (guest, #61103) [Link] (10 responses)

I got that test backwards. It should be:

if ((p1 + length >= p2) || (p2 + length >= p1)) {
crash_and_burn();
}

But goofing the test, doesn't mean the test isn't simple.

Glibc change exposing bugs

Posted Nov 11, 2010 2:12 UTC (Thu) by gus3 (guest, #61103) [Link] (9 responses)

Aaaaand I've still goofed it. I'm not taking into account... well, everything.

I see the actual test in my head, but I can't code it right now due to fatigue. But even with all necessary calculations, being integer math, it'll take no more than a few tens of cycles. Even on a register-starved x86, putting a couple temporary variables on the stack will only pollute the cache, before over-writing the temps anyway. It shouldn't take more than a microsecond to check for overlap.

Glibc change exposing bugs

Posted Nov 11, 2010 7:24 UTC (Thu) by nix (subscriber, #2304) [Link] (6 responses)

A few tens of cycles! That's much longer than an in-cache memcpy() on a small input takes: and most of its inputs are small.

There is absolutely no chance that the glibc devs would ever accept this except in the -lc_g version of the library (which nobody ever uses).

Glibc change exposing bugs

Posted Nov 12, 2010 23:04 UTC (Fri) by gus3 (guest, #61103) [Link] (5 responses)

It shouldn't be difficult to put it in as debugging code, but this isn't normal debugging. The GNU people should own up to having violated the documentation on their code. One year of making noise, or maybe even just six months, should be long enough for developers to clean up their code from this erroneous assumption.

Glibc change exposing bugs

Posted Nov 14, 2010 22:29 UTC (Sun) by nix (subscriber, #2304) [Link] (4 responses)

The GNU people should own up to having violated the documentation on their code.

What on earth? The relevant documentation for memcpy() is ISO C, incorporated by reference into all versions of POSIX.1. This clearly states "If copying takes place between objects that overlap, the behavior is undefined."

This isn't an obscure or hard-to-interpret part of the Standard. Undefined, bang, that's it. Perhaps you are operating under the misapprehension that the linux manpages project, a descriptive effort, not a prescriptive one, is in some way binding on glibc? It isn't. It really isn't. It isn't binding on anything.

Glibc change exposing bugs

Posted Nov 15, 2010 0:00 UTC (Mon) by promotion-account (guest, #70778) [Link] (3 responses)

What on earth? The relevant documentation for memcpy() is ISO C, incorporated by reference into all versions of POSIX.1.

Indeed.

Linux man-pages are only authoritative for the kernel system-calls (more precisely, their glibc thin layer). The rest of the APIs are only included for convenience: they are a secondary source to the primary source references residing in the 'CONFORMING TO' section.

Glibc change exposing bugs

Posted Nov 15, 2010 0:31 UTC (Mon) by nix (subscriber, #2304) [Link] (2 responses)

Linux man-pages are only authoritative for the kernel system-calls (more precisely, their glibc thin layer).

No, even those are descriptive. Perhaps the glibc texinfo documentation would be authoritative for that, if it was ever maintained by anyone. As it is, I think only Ulrich and Roland's brains are authoritative for glibc.

Glibc change exposing bugs

Posted Nov 15, 2010 1:27 UTC (Mon) by promotion-account (guest, #70778) [Link] (1 responses)

I remember finding a good number of system-call manpages discussions in LKML. Otherwise, where should we find documentation for things like futexes, netlink sockets, and the rest?

For good or bad, these manpages are the 'most primary' sources available for such topics, only beside the code.

But unfortunately these man-pages do not always exist. I once had to carefully study the bluez userspace code to know how to best interface with the kernel Bluetooth API (undocumented AF_BLUETOOTH sockets, undocumented netlink interfaces, etc).

Glibc change exposing bugs

Posted Nov 15, 2010 10:38 UTC (Mon) by nix (subscriber, #2304) [Link]

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

Posted Nov 11, 2010 18:52 UTC (Thu) by oak (guest, #2786) [Link] (1 responses)

memmove() has this check you're clamoring for... And if the given areas don't overlap, it calls memcpy().

Glibc change exposing bugs

Posted Nov 15, 2010 0:14 UTC (Mon) by promotion-account (guest, #70778) [Link]

memmove() has this check you're clamoring for... And if the given areas don't overlap, it calls memcpy().

Sometimes even if the areas do overlap, it calls memcpy(). This happens if the library has an internal knowledge about memcpy()'s copying direction.

A common example is having src > dst, copying is forward, and the CPU block transfer unit is smaller than or equal to (src - dst). x86-64 CPUs support copying up-to 8-byte blocks in one opcode (movsq), assuming no floating-point ops in use, which is usually the case with kernel code.

Glibc change exposing bugs

Posted Nov 12, 2010 6:34 UTC (Fri) by jmm82 (guest, #59425) [Link] (1 responses)

That is two tests and one branch.

Glibc change exposing bugs

Posted Nov 12, 2010 22:56 UTC (Fri) by gus3 (guest, #61103) [Link]

Given that the total calculations will be less than one memory page, or one cache line, and the tests can be marked in code as "likely to fail" (that is, no crash and burn), Intel's post-486 processors will already be fetching the normal (everything's OK) code by the time the final test calculations are done. The branch prediction will fail on the crash-and-burn case.