Re: stdbool.h -nostdinc XSA-55 trouble

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 08/08/2013 20:24, Ian Campbell wrote:
> On Thu, 2013-08-08 at 20:05 +0100, Andrew Cooper wrote:
>> On 08/08/2013 18:26, Patrick Welche wrote:
>>> On Thu, Aug 08, 2013 at 05:12:51PM +0100, Ian Campbell wrote:
>>>> (adding Ian J who did most of XSA-55)
>>>> On Thu, 2013-08-08 at 16:47 +0100, Patrick Welche wrote:
>>>>> On Thu, Aug 08, 2013 at 04:30:06PM +0100, Jan Beulich wrote:
>>>>>> No, according to my checking, the --prefix configure option
>>>>>> listed does not correlate with the directory where the header
>>>>>> is found.
>>>>> Yes - I think our emails crossed...
>>>>>
>>>>> The underlying problem is:
>>>>>
>>>>> Linux:   /usr/lib/gcc/i?86-linux-gnu/n.m/include/stdbool.h
>>>>> NetBSD:  /usr/include/stdbool.h
>>>> The hypervisor side, which is where --nostdinc is used, has it's own
>>>> bool_t in asm/types.h. Perhaps libelf (which is supposed to compile for
>>>> both user and hypervisor space) should be using
>>>> #ifdef __XEN__
>>>> #include <asm/types.h>
>>>> #else
>>>> #include <stdbool.h>
>>>> #endif
>>>>
>>>> ?
>>>>
>>>> I'd be a bit concerned about the fact that Xen's bool_t is a typedef for
>>>> char, as opposed to the compilers typedef to _Bool which has special
>>>> meaning. It may not matter in practice but might the fact that _Bool
>>>> canonicalises to 0 or 1 vs. 0 or !0 cause something subtle?
>>> AFAIK
>>>
>>> char c=0;
>>> !c == 1 (true) (rather than 0xff or ~c or whatever - but this is without
>>>                 a "malicious compiler")
>>>
>>> so at a glance, this seems OK. (But then I don't know the original
>>> motivation for replacing bools in XSA-55...)
>>>
>>> Cheers,
>>>
>>> Patrick
>> XSA-55 ended up fighting against the C specification.
>>
>> Under C, the act of creating an invalid pointer is itself undefined.
>> Therefore, taking a base address and adding an offset is possibly
>> undefined, depending on whether the offset lives within the malloc()'d
>> space or not.  As a result, the range check can be optimised away,
>> because if it can be proved to be correct, it will always pass, and if
>> it is isn't the undefined behaviour rules can allow it to also be true.
>>
>> This means that an aggressive optimising compiler can (and, I am
>> informed, does) optimise away range checks, resulting in code which
>> reads as secure, but compiles as insecure.
>>
>> The changes for XSA-55 resulted in exercising as many guarantees from
>> the C standard as much, and working around the rest.  As you might
>> notice from some of the larger patches, structure access (on untrusted
>> structures) is reimplemented as macros and unions, so as to be
>> guaranteed to not be optimised away, even by the most aggressive compilers.
> What does any of that have to do with the use of stdbool?
>
> Ian.
>
>

Sorry - I guess I didn't make that as clear as I was intending to.

"exercising as many guarantees from the C standard"

Part of the proof that the new code was good involved turning the
over-use of ints to other types.  Some to unsigned integers (for array
indicies).  Turning the ints used as booleans to _Bools helped reduce
the noise.  IIRC, there might have been a place where ints (expecting to
be bools) were added to a base, which was functionally broken if the int
contained anything other than 0 or 1.

~Andrew