Re: [PATCH] Upstreaming a large amount of patches

Hi Christian,

I just imported a ton of your patches. Thanks for your work!

> ========================== 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.

Generally speaking, these are all harmless.
As long as the first element of the word is not out of bounds, and the
accesses are aligned, you are good. Access limitations happen on page
boundaries, and you can't cross a page boundary like that.

So it is safe to do this.

However, valgrind will whine. That's why I added a #define
WANT_VALGRIND_SUPPORT, which will detect, at run time, whether we are
running under valgrind, and then switch back to slow and unverbose
versions.

This has not been implemented for all variants, but since for
development I'm only using x86_64, and it's implemented there, and I
can't imagine anybody running valgrind on ARM (it's so slow already!),
there has never been much pressure to support this on the other
routines.

Patches are welcome!

>       Ubuntu's gcc passes -fstack-protector by default, so if dietlibc
>       is compiled without WANT_SSP, it should explicitly pass
>       -fno-stack-protector to the compiler.

No it should not.
It should for the one start file that initializes the stack cookie.
This is already done, with a facepalm comment for good measure :)

>       This has been carried in Debian because Debian has historically
>       disabled SSP due to toolchain issues; this will not be the case
>       any more for future versions (because recent toolchains aren't
>       buggy in this regard anymore and we want SSP to make attacks more
>       difficult), so it's unclear what should happen with this patch
>       now, especially since it just presupposes that the compiler
>       understands the flag -fno-stack-protector... OTOH, people
>       compiling dietlibc on Ubuntu themselves from upstream will run
>       into trouble without this patch if they disable WANT_SSP...

I don't see why they should run into trouble.

> https://gist.githubusercontent.com/chris-se/e35381c75583a4c4bce2/raw/8bf327a048f396220c48fb4ffbd504d12b042007/unclear-A02-native-x32.patch

>       Debian has a native x32 port (currently still unofficial), where
>       all binaries of the system can be x32. In that case, it kind of
>       makes sense to provide x32 support not only as an "add-on" to
>       x86_64, but also as its own architecture. Unfortunately, there is
>       no way to distinguish x86_64 from x32 based on the kernel (that
>       is identical; uname -m returns x86_64 in both cases), because
>       only the userland is different.

>       However, with the patch I provided here one can specify
>          make MYARCH=x32
>       in order to tell the build system that a full x32 build is
>       wanted. And it also has the diet binary assume x32 by default if
>       if the diet binary was compiled natively.

>       So while it doesn't help to support native x32 automatically,
>       hence I moved this patch into the "unclear" category, the
>       maintenance burden is basically zero, so unless you have a better
>       idea how to support this, I'd be grateful if you could apply the
>       patch.

>       Patch author: Christian Seiler <[email protected]>

I don't think it makes sense to have this kind of thing in the mainline
dietlibc, but feel free to keep on doing this for your x32 distro.

If your distro is x32 by default, I assume that means you pass -mx32 in
gcc by default, too? Shouldn't dietlibc compile and work just fine out
of the box then? Wouldn't it be just a cosmetic issue what the directory
is called?

Felix