Re: [PATCH] Upstreaming a large amount of patches

On 03/22/2016 09:28 PM, Christian Seiler wrote:
> Hi,
>
> I've started helping out with the Debian package of dietlibc, because
> the current package in unstable is ~ 4 years old and really needs an
> update.
>
> I've already uploaded a version to Debian's experimental repository
> that is based on a recent CVS snapshot, but one thing I noticed is that
> there are a _lot_ of local patches in the Debian package - and the vast
> majority of them are not Debian-specific.
A big THANK YOU for this!

> ...
>
> ========================== UNCLEAR STUFF ==============================
>
>   * Out of bounds memory access with fast string functions without
>     WANT_SMALL_STRING_ROUTINES
>
>        The following string functions have variants that read up to 7
>        bytes beyond possible buffer sizes. Specifically, this affects:
>
>         - arm/strcpy.S
>         - arm/strlen.S
>         - lib/strcmp.c
>         - lib/strcpy.c
>         - lib/strlen.c
>
>        Not affected are:
>
>         - lib/strcat.c
>         - lib/strncat.c
>         - lib/strrchr.c
>         - lib/strchr.c
>                manual loop unrolling
>
>         - lib/memcpy.c
>                only uses word-size access as long as at least a word is
>                left to be copied
>
>        This out-of-bounds access is problematic because some memory
>        allocators enforce buffer boundaries explicitly; and it also
>        generates tons of false positives with valgrind and similar
>        tools.
>
>        The package in Debian has had a patch for the last 5 years that
>        removes the string function variants that perform out of bounds
>        access. I've looked at the patch and it just removes stuff
>        between the #ifndef WANT_SMALL_STRING_ROUTINES and the #endif in
>        those cases, but I feel really uncomfortable carrying such a
>        patch going forward (I have the OpenSSL situation from a couple
>        of years ago in the back of my mind), so I'd rather have this
>        upstream. Of course, you might want to keep the faster function
>        variants regardless, but may I at least suggest adding another
>        define WANT_FAST_OOB_STRING_FUNCTIONS to enable them. If you are
>        agreeable to this, I can write a patch that does just that.
>
After a quick look at strlen.c and strcpy.c
this looks to me like it's ok, there are a few well known tricks
to speed up string operations using whatever is the native word size;
the basic idea being that, yes, they WILL read/write up to seven excess 
bytes on amd64, but it's harmless because
1. the reads will never cross a page boundary, since pages are aligned;
    it will cross a malloc boundary, but always staying in the same page.
2. at least in strcpy, the writes use char pointers when a \0 is found
    - look at the hellish "if (((l - MKW(0x1ul)) & ~l) & MKW(0x80ul))"
      and figure out what it does :D

There was an AMAZING stackoverflow answer explaining how it works, but
I can't find it atm.

Imho it would be worth to add a note to maintainers, and for maintainers
to tell upstream when they have these problems!