Undefined behavior in glibc -- was: Re: [PATCH][BZ 17979][BZ 17721] Fix issues with sys/cdefs.h and uchar.h when using non-gcc compiler.

  Joseph,

thanks a lot for the detailed reply. My comments are below.

On 2016-01-31 19:12, Joseph Myers wrote:
> On Sat, 30 Jan 2016, Alexander Cherepanov wrote:
>
>> But we can consider something much more real: glibc code contains many cases
>> of undefined behavior (according to C11). I consider it bugs unwanted in
>> production (maybe with the exception of strlen) but it's not clear to me what
>> is the POV of the project on this. E.g., are bug reports about UB are welcome?
>
> It's an intrinsic part of implementing the C library that it involves
> doing things that are not defined in ISO C (the ISO C library cannot all
> be implemented in ISO C).

But those parts which cannot be implemented in ISO C would be 
implemented in asm? AIUI there is no much magic in compiling C parts of 
glibc so they should obey the common rules?

> If something involves undefined behavior in GNU C,

GNU C is ISO C (whatever it means:-) + GCC implementation-defined 
behavior as described in [1] + GCC extensions[2], right?

[1] https://gcc.gnu.org/onlinedocs/gcc/C-Implementation.html
[2] https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html

> on systems with the
> properties that hold for all systems supported by glibc (e.g. 32-bit int,
> 32-bit or 64-bit long and pointers), for inputs to library functions where
> the library function semantics don't involve undefined behavior, taking
> due account of both separate compilation and use of asm to reduce the
> information available to the compiler in certain places and so make things
> not undefined that would otherwise be undefined, then I'd consider bug
> reports appropriate.  Examples of this include buffer overruns and signed
> integer arithmetic overflow.  I think that applies even if there is no
> plausible optimization that could result in the signed integer arithmetic
> overflow causing problems.  Of course, patches are even better.

This looks reasonable but I guess there supposed to be some exceptions? 
E.g.:

1) strlen and other string functions read memory by chunks which is 
aliasing violation;
2) strlen accesses the OOB memory.

AIUI for 1) there is may_alias gcc attribute but it almost never used in 
glibc. There are no tools in GNU C to make 2) well-defined but the 
speed-up is remarkable (Rich Felker mentioned 3x difference in speed in 
musl strlen on his hw).

Documenting all the exceptions would be nice. Then, ASan is in glibc 
todo so a clean version of strlen has to be added.

> If something is fully defined in GNU C, but undefined, unspecified or
> implementation-defined in ISO C, I don't think bug reports are
> appropriate.  E.g., it's not a bug for glibc code to rely on signed shifts
> by a nonnegative amount that's less than the width of the type, including
> the aspects of those that are not defined by ISO C, or on the
> fully-defined nature of conversions of out-of-range integer values to
> signed integer types.  Some such cases in code shared with gnulib (or
> other external sources) may still be gnulib bugs where they aren't glibc
> bugs, since gnulib is meant to be much more portable.  And there may be
> cases where you could argue that eliminating such usages is a desirable
> *cleanup* in glibc - e.g., that the code is cleaner when it uses unsigned
> types for bitwise operations.  But in such cases there is not bug and so
> patches should be sent without any bug reports in Bugzilla (or more
> general groups of issues identified in the list of possible cleanups in
> the todo list on the wiki).

Ok

Making glibc compilable by clang would be nice too (and it's in the 
glibc todo list).

> Where something depends on a property common to all systems supported by
> glibc, again it may be a useful *cleanup* to make this more explicit in
> the code, without there being any *bug* that's appropriate to report to
> Bugzilla.  E.g., if code is using int in a way that requires it to be
> exactly 32-bit, changing the code to use int32_t is a sensible cleanup; if
> it really does mean int but is hardcoding the value of INT_MAX, making it
> use INT_MAX is a sensible cleanup; if it's using long as a pointer-sized
> integer type, changing it to use intptr_t is a sensible cleanup.

Ok

> Where glibc code relies on separate compilation to avoid undefined
> behavior, this is not a bug; use of LTO on glibc's own code is not
> supported.  For example, where functions with different C types but
> compatible ABIs are aliased, or a function is otherwise defined with a
> different type from that used when it is called.  Similarly, asm may be
> used to limit code movement, and that could e.g. mean that aliasing is not
> undefined behavior where it would otherwise be undefined.

Not sure what you mean. Is it specific to calling functions or it's 
supposed to cover the case of strlen too?

I don't see cleaning glibc for LTO in todo list in the wiki. Shouldn't 
it be there?

> Of course, if the semantics for a function say that certain inputs result
> in undefined behavior, it does not matter if C-level undefined behavior
> occurs within that function for those inputs.

Sure.

Well, before I go and file the bugs, perhaps it's worth skimming through 
the list. It's mainly invalid pointer arithmetic. GNU C seems not to 
deviate from ISO C in this topic. Should the issues be grouped by type, 
by file, by subdir or in some other way?

Pointer wrapping
----------------

Likely to be miscompiled in the future. Already reported:

https://sourceware.org/bugzilla/show_bug.cgi?id=19411
https://sourceware.org/bugzilla/show_bug.cgi?id=19412
https://sourceware.org/bugzilla/show_bug.cgi?id=19413

Pointers pointing past an array
-------------------------------

I guess there are a lot of these but searching for them is not easily 
automated so the list below is probably just the tip of the iceberg and 
maybe not even accurate.

Sometimes it could be proved that the amount of "overflow" is small and, 
thus, it should be safe in practice (given that an object usually could 
not be allocated near the end of the address space).

https://sourceware.org/git/?p=glibc.git;a=blob;f=resolv/ns_name.c;h=f355cf34443c46d091157ed06f4ef8487214bf10;hb=HEAD#l548
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getai.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l113
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getgr_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l240
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_gethst_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l215
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_gethst_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l370
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getpw_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l168
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getserv_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l139
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getserv_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l292
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_initgroups.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l73

Pointers pointing before an array
---------------------------------

Should be safe in practice?

https://sourceware.org/git/?p=glibc.git;a=blob;f=wcsmbs/wcsncat.c;h=9b22764ca13b16a96b6f32636072203658950fdd;hb=HEAD#l39
https://sourceware.org/git/?p=glibc.git;a=blob;f=wcsmbs/wcsncpy.c;h=cd90024e4410cf06cc1083ec10d6f677f18f3e92;hb=HEAD#l32
https://sourceware.org/git/?p=glibc.git;a=blob;f=string/memcmp.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l131
https://sourceware.org/git/?p=glibc.git;a=blob;f=string/wordcopy.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l42
etc.

Capping pointers at the end of the address space
------------------------------------------------

Some functions in C11 have a parameter which limits a number of 
characters written. IMHO it's clear that C11 doesn't intend this 
parameter to be a real size of output buffer and, thus, it could validly 
be SIZE_MAX. But we have seen differing opinions in the recent thread 
about strlcpy. Perhaps you can clear the question.

Anyway, glibc inner parts use a pointer to the end of the buffer and 
upper-level functions have to check for pointer wrapping and to cap this 
pointer at the end of the address space.

1. Functions like snprintf cap the pointer in _IO_str_init_static_internal.

2. Functions like wcsrtombs have to cap __outbufend in struct 
__gconv_step_data.

The capping itself is not UB (if done right) but later pointer 
comparisons with the resulting pointer are UB. Probably safe in practice 
as it's similar to [7/15] in 
https://gcc.gnu.org/ml/gcc/2015-04/msg00325.html .

API change required? :-)

Then, there are less important cases:

https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_gethst_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l169
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getai.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l68
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getgr_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l105
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getpw_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l100
https://sourceware.org/git/?p=glibc.git;a=blob;f=nscd/nscd_getserv_r.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l113

and probably others.

Subtracting NULL from a pointer
-------------------------------

Plausible optimization: this is unconditional UB, hence mark the code as 
unreachable and remove everything around.

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/sigstack.c;h=b134d8e5b9655aec4afea961e1eb63f558be62c1;hb=HEAD#l45
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/profil.c;h=23e601c6c2d5a677a778df65c9088f34ff1b7a04;hb=HEAD#l40
35 other cases are in the attached subtract-null.patch (generated).

Structs at NULL
---------------

Plausible optimization: this is unconditional UB, hence mark the code as 
unreachable and remove everything around.

https://sourceware.org/git/?p=glibc.git;a=blob;f=argp/argp-parse.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l81
https://sourceware.org/git/?p=glibc.git;a=blob;f=include/list.h;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l94
https://sourceware.org/git/?p=glibc.git;a=blob;f=intl/bindtextdom.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l44
https://sourceware.org/git/?p=glibc.git;a=blob;f=intl/dcigettext.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l120
https://sourceware.org/git/?p=glibc.git;a=blob;f=intl/dcigettext.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l125
https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl_db/db_info.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l76
https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/glob.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l151
https://sourceware.org/git/?p=glibc.git;a=blob;f=resolv/nss_dns/dns-network.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l241
https://sourceware.org/git/?p=glibc.git;a=blob;f=stdio-common/reg-modifier.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l58
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/ia64/sys/ucontext.h;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l39

Pointers used after free (without dereferencing)
------------------------------------------------

https://sourceware.org/git/?p=glibc.git;a=blob;f=libio/fileops.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l1063
(IIUC conditions "fp->_IO_read_base != NULL" and "!_IO_in_backup (fp)" 
should be swapped.)

https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-obstack.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l25
https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-malloc-backtrace.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l30
https://sourceware.org/git/?p=glibc.git;a=blob;f=io/pwd.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l41

Memory allocations without check
--------------------------------

Not UB but while we are at it...

AIUI the policy in glibc is to check results of malloc no matter how 
small the requested amount of memory is. Right?

So mallocs like in https://sourceware.org/bugzilla/show_bug.cgi?id=19416 
should be reported?
  

On Wed, 3 Feb 2016, Alexander Cherepanov wrote:

> But those parts which cannot be implemented in ISO C would be implemented in
> asm? AIUI there is no much magic in compiling C parts of glibc so they should

No, not necessarily.  Written in GNU C with a good understanding of what 
transformations are actually possible given the code visible to the 
compiler and the rules of GNU C.

> > If something involves undefined behavior in GNU C,
> 
> GNU C is ISO C (whatever it means:-) + GCC implementation-defined behavior as
> described in [1] + GCC extensions[2], right?

Plus undocumented features and a good understanding of aspects of how C 
relates to the underlying machines (that is, you cannot assume e.g. that a 
C addition corresponds to a machine add instruction, but you can assume 
that memory is made of pages and that if it's not visible to the compiler 
how memory was allocated, having particular access to any byte in a page 
means having such access to all bytes in that page, subject to avoiding 
data races on write).  Cf 
<https://www.cl.cam.ac.uk/~pes20/cerberus/notes50-2015-05-24-survey-discussion.html> 
(but the non-ISO aspects of GNU C as used in low-level system software 
such as glibc and the Linux kernel should be assumed to be much more 
wide-ranging than the issues discussed there).

> 1) strlen and other string functions read memory by chunks which is aliasing
> violation;
> 2) strlen accesses the OOB memory.

Both of those fall under deliberate use of separate compilation, i.e. it's 
not visible to the compiler what the original effective type of the memory 
was or its original size, so a whole page can be accessed provided the 
types used don't involve aliasing violations within the source visible to 
the compiler.

> > Where glibc code relies on separate compilation to avoid undefined
> > behavior, this is not a bug; use of LTO on glibc's own code is not
> > supported.  For example, where functions with different C types but
> > compatible ABIs are aliased, or a function is otherwise defined with a
> > different type from that used when it is called.  Similarly, asm may be
> > used to limit code movement, and that could e.g. mean that aliasing is not
> > undefined behavior where it would otherwise be undefined.
> 
> Not sure what you mean. Is it specific to calling functions or it's supposed
> to cover the case of strlen too?

It covers strlen.  strlen is well-defined in isolation, so can be presumed 
to be compiled to an ABI-conforming object file, and such an object file, 
if it would work for programs where it would be valid for that strlen 
implementation to be part of the program (under a different name), would 
also work for other programs that are valid with ISO C strlen.

> I don't see cleaning glibc for LTO in todo list in the wiki. Shouldn't it be
> there?

The todo list basically has ideas that one person once thought might be 
useful.  In general, you can't assume that things there have consensus, 
and nor do you need consensus to put something there.  I think 
facilitating LTO of the standard C library is a very hard whole-system 
issue, not something that could be considered for glibc in isolation; 
you'd probably need several different forms of source annotation that act 
as barriers to particular forms of LTO, saying that calls to a function 
can only be optimized with reference to its semantics and not with 
reference to the contents of a particular implementation of it.

> Well, before I go and file the bugs, perhaps it's worth skimming through the
> list. It's mainly invalid pointer arithmetic. GNU C seems not to deviate from
> ISO C in this topic. Should the issues be grouped by type, by file, by subdir
> or in some other way?

I think such cases should generally be considered bugs except for limited 
cases such as string functions that deliberately work in terms of pages or 
aligned units smaller than pages, and so may go slightly outside the 
original object (before or after) but obviously cannot wrap around the 
address space to affect the results of comparisons.

The natural division is based on whether it might make sense to fix two 
issues separately - so any issues in parts of glibc for which different 
people might have expertise, or that differ in how clear it is that there 
is a problem or how clear it is what the correct fix would be, should be 
filed separately.  Of course, it's even better if fixes (following the 
contribution checklist) for such issues are submitted as well.

> Capping pointers at the end of the address space
> ------------------------------------------------
> 
> Some functions in C11 have a parameter which limits a number of characters
> written. IMHO it's clear that C11 doesn't intend this parameter to be a real
> size of output buffer and, thus, it could validly be SIZE_MAX. But we have
> seen differing opinions in the recent thread about strlcpy. Perhaps you can
> clear the question.

Given the dispute in the Austin Group over snprintf, a C11 DR is clearly 
needed.

> Structs at NULL
> ---------------
> 
> Plausible optimization: this is unconditional UB, hence mark the code as
> unreachable and remove everything around.

I think such offsetof-type code should be considered valid GNU C, although 
it would still be better to use offsetof.  At least some are inside 
"#ifndef offsetof" (some of these examples are code shared with gnulib or 
gettext) and so obviously not bugs in glibc (given that <stddef.h> is 
included).

> Pointers used after free (without dereferencing)
> ------------------------------------------------
> 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=libio/fileops.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l1063
> (IIUC conditions "fp->_IO_read_base != NULL" and "!_IO_in_backup (fp)" should
> be swapped.)
> 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-obstack.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l25
> https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-malloc-backtrace.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l30
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/pwd.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l41

I'd say such comparisons should be avoided, but that's more a cleanup than 
a bug fix.

> Memory allocations without check
> --------------------------------
> 
> Not UB but while we are at it...
> 
> AIUI the policy in glibc is to check results of malloc no matter how small the
> requested amount of memory is. Right?

Yes.

> So mallocs like in https://sourceware.org/bugzilla/show_bug.cgi?id=19416
> should be reported?

Yes.
  

On 2016-02-03 16:07, Joseph Myers wrote:
>>> If something involves undefined behavior in GNU C,
>>
>> GNU C is ISO C (whatever it means:-) + GCC implementation-defined behavior as
>> described in [1] + GCC extensions[2], right?
>
> Plus undocumented features

The moment you say "undocumented" everything falls apart. One cannot be 
sure whether some observed behavior is an undocumented feature or 
something that will be broken by the next gcc release.

> and a good understanding of aspects of how C
> relates to the underlying machines (that is, you cannot assume e.g. that a
> C addition corresponds to a machine add instruction, but you can assume
> that memory is made of pages and that if it's not visible to the compiler
> how memory was allocated, having particular access to any byte in a page
> means having such access to all bytes in that page, subject to avoiding
> data races on write).

1. This rules out all modern hardening/debugging solutions like 
Valgrind, ASan, Intel MPX, CHERI (don't know if the latter is relevant 
for glibc).

2. You seem to be concerned with crashes only but there are other 
problems too, e.g., info leaks (think Heartbleed). Saying that the code 
is free to access any accessible memory means that one cannot catch info 
leaks automatically.

> Cf
> <https://www.cl.cam.ac.uk/~pes20/cerberus/notes50-2015-05-24-survey-discussion.html>
> (but the non-ISO aspects of GNU C as used in low-level system software
> such as glibc and the Linux kernel should be assumed to be much more
> wide-ranging than the issues discussed there).

Yes, I have seen it. I have even linked to your reply to this survey in 
my last email.

>> 1) strlen and other string functions read memory by chunks which is aliasing
>> violation;
>> 2) strlen accesses the OOB memory.
>
> Both of those fall under deliberate use of separate compilation, i.e. it's
> not visible to the compiler what the original effective type of the memory
> was or its original size, so a whole page can be accessed provided the
> types used don't involve aliasing violations within the source visible to
> the compiler.

I thought that this falls under "there is no plausible optimization [due 
to the info not available to a compiler]". Well, ok.

Then let's look at strlen (&co.) from another angle. It starts accessing 
its parameter as chars and then continues accessing it as longs, like this:

   ...
   char *p = ...;
   *p++;
   *(long *)p;

Is there a situation where this is valid in GNU C? For ISO C the 
question seems quite moot but I think Question 16 in DR 017 plus DR 260 
say that it's not valid. Maybe there are no restrictions in GNU C on 
intra-object pointer arithmetic / accesses at all (apart from aliasing 
rules)?

>> I don't see cleaning glibc for LTO in todo list in the wiki. Shouldn't it be
>> there?
>
> The todo list basically has ideas that one person once thought might be
> useful.  In general, you can't assume that things there have consensus,
> and nor do you need consensus to put something there.  I think
> facilitating LTO of the standard C library is a very hard whole-system
> issue, not something that could be considered for glibc in isolation;
> you'd probably need several different forms of source annotation that act
> as barriers to particular forms of LTO, saying that calls to a function
> can only be optimized with reference to its semantics and not with
> reference to the contents of a particular implementation of it.

Isn't noinline+noclone enough for this right now?

>> Well, before I go and file the bugs, perhaps it's worth skimming through the
>> list. It's mainly invalid pointer arithmetic. GNU C seems not to deviate from
>> ISO C in this topic. Should the issues be grouped by type, by file, by subdir
>> or in some other way?
>
> I think such cases should generally be considered bugs except for limited
> cases such as string functions that deliberately work in terms of pages or
> aligned units smaller than pages, and so may go slightly outside the
> original object (before or after) but obviously cannot wrap around the
> address space to affect the results of comparisons.

Ok

> The natural division is based on whether it might make sense to fix two
> issues separately - so any issues in parts of glibc for which different
> people might have expertise, or that differ in how clear it is that there
> is a problem or how clear it is what the correct fix would be, should be
> filed separately.  Of course, it's even better if fixes (following the
> contribution checklist) for such issues are submitted as well.

Ok

>> Capping pointers at the end of the address space
>> ------------------------------------------------
>>
>> Some functions in C11 have a parameter which limits a number of characters
>> written. IMHO it's clear that C11 doesn't intend this parameter to be a real
>> size of output buffer and, thus, it could validly be SIZE_MAX. But we have
>> seen differing opinions in the recent thread about strlcpy. Perhaps you can
>> clear the question.
>
> Given the dispute in the Austin Group over snprintf, a C11 DR is clearly
> needed.

Ok

>> Structs at NULL
>> ---------------
>>
>> Plausible optimization: this is unconditional UB, hence mark the code as
>> unreachable and remove everything around.
>
> I think such offsetof-type code should be considered valid GNU C, although

Ok

> it would still be better to use offsetof.  At least some are inside
> "#ifndef offsetof" (some of these examples are code shared with gnulib or
> gettext) and so obviously not bugs in glibc (given that <stddef.h> is
> included).

Yeah, and I managed to include some that are used to get a size of a 
field instead of an offset and so are fine even in ISO C. Have to filter 
them out in the future.

>> Pointers used after free (without dereferencing)
>> ------------------------------------------------
>>
>> https://sourceware.org/git/?p=glibc.git;a=blob;f=libio/fileops.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l1063
>> (IIUC conditions "fp->_IO_read_base != NULL" and "!_IO_in_backup (fp)" should
>> be swapped.)

FTR: this one seems to be a false. fp->_IO_read_base could be accessed 
on line 1063 after a free on line 1020 but it's set inside _IO_setg on 
line 1025.

>> https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-obstack.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l25
>> https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/tst-malloc-backtrace.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l30
>> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/pwd.c;hb=5163b4b76f61e361f0f4bbe3b96732b12e5c9b1a#l41
>
> I'd say such comparisons should be avoided, but that's more a cleanup than
> a bug fix.

BTW is there a formal method to tell test files from important files? I 
see .../tst-* and .../test-* files but io/pwd.c looks like a test too.
  

On Fri, 5 Feb 2016, Alexander Cherepanov wrote:

> Then let's look at strlen (&co.) from another angle. It starts accessing its
> parameter as chars and then continues accessing it as longs, like this:
> 
>   ...
>   char *p = ...;
>   *p++;
>   *(long *)p;
> 
> Is there a situation where this is valid in GNU C? For ISO C the question

If the underlying object has effective type an array of longs it seems 
valid to me.

> BTW is there a formal method to tell test files from important files? I see
> .../tst-* and .../test-* files but io/pwd.c looks like a test too.

You need to look at what files are listed in the relevant makefile 
variables.
  

On 05.02.2016 21:38, Joseph Myers wrote:
> On Fri, 5 Feb 2016, Alexander Cherepanov wrote:
>
>> Then let's look at strlen (&co.) from another angle. It starts accessing its
>> parameter as chars and then continues accessing it as longs, like this:
>>
>>    ...
>>    char *p = ...;
>>    *p++;
>>    *(long *)p;
>>
>> Is there a situation where this is valid in GNU C? For ISO C the question
>
> If the underlying object has effective type an array of longs it seems
> valid to me.

Ok, then there are two cases here. If you take an address of an element 
of this array and convert it to char* then you cannot go outside of this 
element -- this is the essence of Q16 in DR 017.

If you take an address of the array itself then you can access any of 
its bytes but I don't think the standard permits you to go back from 
working with chars to working with longs. Roughly speaking, the 
structure of the object is forgotten. While you stay at the beginning of 
the object you can go back -- it's a general rule: you can convert 
unchanged pointers forth and back freely (modulo alignment). But if you 
move from the beginning then you lose this freedom. The standard doesn't 
describe going from an unrelated pointer to char to a pointer to an 
(sub)object.
  

On Fri, 5 Feb 2016, Alexander Cherepanov wrote:

> If you take an address of the array itself then you can access any of its
> bytes but I don't think the standard permits you to go back from working with
> chars to working with longs. Roughly speaking, the structure of the object is
> forgotten. While you stay at the beginning of the object you can go back --
> it's a general rule: you can convert unchanged pointers forth and back freely
> (modulo alignment). But if you move from the beginning then you lose this
> freedom. The standard doesn't describe going from an unrelated pointer to char
> to a pointer to an (sub)object.

I think going from the pointer to char back to a pointer to long is valid 
in GNU C and in common usage C, provided you never access the same memory 
with different non-character types (other than signed/unsigned variations) 
in ways that would require a union to do without conversions between 
pointer types.
  

On 2016-02-06 01:35, Joseph Myers wrote:
> On Fri, 5 Feb 2016, Alexander Cherepanov wrote:
>
>> If you take an address of the array itself then you can access any of its
>> bytes but I don't think the standard permits you to go back from working with
>> chars to working with longs. Roughly speaking, the structure of the object is
>> forgotten. While you stay at the beginning of the object you can go back --
>> it's a general rule: you can convert unchanged pointers forth and back freely
>> (modulo alignment). But if you move from the beginning then you lose this
>> freedom. The standard doesn't describe going from an unrelated pointer to char
>> to a pointer to an (sub)object.
>
> I think going from the pointer to char back to a pointer to long is valid
> in GNU C and in common usage C, provided you never access the same memory
> with different non-character types (other than signed/unsigned variations)
> in ways that would require a union to do without conversions between
> pointer types.

Not sure this plays well with other principles. AIUI (from [1], [2], [2] 
etc.) you want to retain the freedom in gcc to optimize based on Q16 in 
DR 017. But going through char* seems to permit cheating. Suppose you 
have a 2d array and a pointer to char which points to the boundary 
between two rows. What you get after converting it to a pointer to the 
type of the elements -- a pointer pointing past the end of the previous 
row or a pointer to the first element of the next row? What are the 
limits for pointer arithmetic with this pointer -- one row, two rows, 
whole array?

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41630#c8
[2] https://gcc.gnu.org/ml/gcc/2001-10/msg00820.html
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54136#c5

I've got a question regarding separate compilation etc. What's the 
problem with explicit_bzero then? Just compile it separately and no 
barriers are required, right?

A couple of additions to my previous emails.

On 2016-02-03 15:29, Alexander Cherepanov wrote:
 > Pointer wrapping
 > ----------------
 >
 > Likely to be miscompiled in the future. Already reported:
 >
 > https://sourceware.org/bugzilla/show_bug.cgi?id=19411
 > https://sourceware.org/bugzilla/show_bug.cgi?id=19412
 > https://sourceware.org/bugzilla/show_bug.cgi?id=19413

I should have included in the list the bug report by Pascal Cuoq:

https://sourceware.org/bugzilla/show_bug.cgi?id=19391

It motivated me to look for other cases of checks for pointer wrapping 
in glibc. There are links to it in my PRs but I think it should 
explicitly mentioned in this thread too.

On 2016-02-05 21:24, Alexander Cherepanov wrote:
 > 1. This rules out all modern hardening/debugging solutions like
 > Valgrind, ASan, Intel MPX, CHERI (don't know if the latter is relevant
 > for glibc).

I guess everybody here is familiar with ASan as used for debugging but 
perhaps it's worth mentioning that there are at least two ongoing 
projects[1][2] working on full systems using ASan (and, for the second 
one, UBSan) for hardening in production. The specific merits are yet to 
be assessed but the interest is definitely there.

[4] 
https://blog.hboeck.de/archives/879-Safer-use-of-C-code-running-Gentoo-with-Address-Sanitizer.html
[5] 
http://balintreczey.hu/blog/proposing-amd64-hardened-architecture-for-debian/
  

On 10/02/16 03:44, Alexander Cherepanov wrote:
> On 2016-02-05 21:24, Alexander Cherepanov wrote:
>> 1. This rules out all modern hardening/debugging solutions like
>> Valgrind, ASan, Intel MPX, CHERI (don't know if the latter is relevant
>> for glibc).
> 
> I guess everybody here is familiar with ASan as used for debugging but perhaps it's worth mentioning that there
> are at least two ongoing projects[1][2] working on full systems using ASan (and, for the second one, UBSan) for
> hardening in production. The specific merits are yet to be assessed but the interest is definitely there.
> 
> [4] https://blog.hboeck.de/archives/879-Safer-use-of-C-code-running-Gentoo-with-Address-Sanitizer.html
> [5] http://balintreczey.hu/blog/proposing-amd64-hardened-architecture-for-debian/
> 

well, address sanitizer was not designed for hardening, there
are ways to fix that, but if such projects start to misuse
asan then it will become harder to fix.

in the glibc context, i think there are only a handful of
cases when the libc has excuse to make assumptions beyond
iso c, and the rest should be clean c code.

but even if the code is fixed, hardening the libc sounds
problematic: on 32bit systems the address space is a scarce
resource and on 64bit systems the shadow map breaks reliable
systems with overcommit off.

(ubsan, i believe, is fine because it works without a runtime,
when it only traps and does not depend on unsafe environment.
unfortunately it has false positives in gcc:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63303

outside of the libc, most of the problems with the sanitizers
are due to their interactions with the c runtime and dynamic
linker through various unreliable hacks that can break at
any libc update and may provide new attack surfaces.)
  

On Wed, 10 Feb 2016, Alexander Cherepanov wrote:

> > I think going from the pointer to char back to a pointer to long is valid
> > in GNU C and in common usage C, provided you never access the same memory
> > with different non-character types (other than signed/unsigned variations)
> > in ways that would require a union to do without conversions between
> > pointer types.
> 
> Not sure this plays well with other principles. AIUI (from [1], [2], [2] etc.)
> you want to retain the freedom in gcc to optimize based on Q16 in DR 017. But
> going through char* seems to permit cheating. Suppose you have a 2d array and
> a pointer to char which points to the boundary between two rows. What you get
> after converting it to a pointer to the type of the elements -- a pointer
> pointing past the end of the previous row or a pointer to the first element of
> the next row? What are the limits for pointer arithmetic with this pointer --
> one row, two rows, whole array?

This is not relevant to the string functions since it's not visible to 
them if a subobject was used to contstruct the pointer passed in.

> I've got a question regarding separate compilation etc. What's the problem
> with explicit_bzero then?

It needs *consensus* on the appropriateness of the patch as it exists now, 
notwithstanding the various ways in which, absent new compiler features, 
other copies of the zeroed memory may still be present and accessible to 
the process if subsequently compromised.

> Just compile it separately and no barriers are
> required, right?

In my view, yes (the arguments about barriers are irrelevant).